[dpdk-dev,RFC,1/2] vhost: make capabilities configurable

Message ID 1514000203-69699-2-git-send-email-zhihong.wang@intel.com (mailing list archive)
State Superseded, 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 Dec. 23, 2017, 3:36 a.m. UTC
  This patch makes vhost device capabilities configurable to adopt various
engines. Such capabilities include supported features, protocol features,
queue number. APIs are introduced to let app configure these capabilities.

Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
---
 lib/librte_vhost/rte_vhost.h  | 50 ++++++++++++++++++++++++++++
 lib/librte_vhost/socket.c     | 77 +++++++++++++++++++++++++++++++++++++++++++
 lib/librte_vhost/vhost_user.c | 48 ++++++++++++++++++++-------
 3 files changed, 164 insertions(+), 11 deletions(-)
  

Comments

Maxime Coquelin Jan. 30, 2018, 2:59 p.m. UTC | #1
Hi Zhihong

On 12/23/2017 04:36 AM, Zhihong Wang wrote:
> This patch makes vhost device capabilities configurable to adopt various
> engines. Such capabilities include supported features, protocol features,
> queue number. APIs are introduced to let app configure these capabilities.

Why does the vDPA driver need to mask protocol features?

Maxime
  
Zhihong Wang Jan. 31, 2018, 5:37 a.m. UTC | #2
Hi Maxime,

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

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

> Sent: Tuesday, January 30, 2018 11:00 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 RFC 1/2] vhost: make capabilities configurable

> 

> Hi Zhihong

> 

> On 12/23/2017 04:36 AM, Zhihong Wang wrote:

> > This patch makes vhost device capabilities configurable to adopt various

> > engines. Such capabilities include supported features, protocol features,

> > queue number. APIs are introduced to let app configure these capabilities.

> 

> Why does the vDPA driver need to mask protocol features?


Different vhost devices may support different combinations of protocol
features, e.g. One may not support MQ, or RARP. So it should be reported
by the device.

Thanks
-Zhihong

> 

> Maxime
  

Patch

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index f653644..17b4c6d 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -209,6 +209,56 @@  int rte_vhost_driver_unregister(const char *path);
 int rte_vhost_driver_set_features(const char *path, uint64_t features);
 
 /**
+ * Get the protocol feature bits before feature negotiation.
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @param protocol_features
+ *  A pointer to store the queried protocol feature bits
+ * @return
+ *  0 on success, -1 on failure
+ */
+int rte_vhost_driver_get_protocol_features(const char *path,
+		uint64_t *protocol_features);
+
+/**
+ * Set the protocol feature bits the vhost-user driver supports.
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @param protocol_features
+ *  Supported protocol features
+ * @return
+ *  0 on success, -1 on failure
+ */
+int rte_vhost_driver_set_protocol_features(const char *path,
+		uint64_t protocol_features);
+
+/**
+ * Get the queue number before feature negotiation.
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @param queue_num
+ *  A pointer to store the queried queue number
+ * @return
+ *  0 on success, -1 on failure
+ */
+int rte_vhost_driver_get_queue_num(const char *path, uint16_t *queue_num);
+
+/**
+ * Set the queue number the vhost-user driver supports.
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @param queue_num
+ *  Supported queue number
+ * @return
+ *  0 on success, -1 on failure
+ */
+int rte_vhost_driver_set_queue_num(const char *path, uint16_t queue_num);
+
+/**
  * Enable vhost-user driver features.
  *
  * Note that
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 422da00..742f772 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -78,7 +78,10 @@  struct vhost_user_socket {
 	 * features negotiation.
 	 */
 	uint64_t supported_features;
+	uint64_t supported_protocol_features;
 	uint64_t features;
+	uint64_t protocol_features;
+	uint16_t queue_num;
 
 	struct vhost_device_ops const *notify_ops;
 };
@@ -613,6 +616,75 @@  rte_vhost_driver_get_features(const char *path, uint64_t *features)
 	}
 }
 
+int rte_vhost_driver_set_protocol_features(const char *path,
+		uint64_t protocol_features)
+{
+	struct vhost_user_socket *vsocket;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket) {
+		vsocket->supported_protocol_features = protocol_features;
+		vsocket->protocol_features = protocol_features;
+	}
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	return vsocket ? 0 : -1;
+}
+
+int
+rte_vhost_driver_get_protocol_features(const char *path,
+		uint64_t *protocol_features)
+{
+	struct vhost_user_socket *vsocket;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		*protocol_features = vsocket->protocol_features;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	if (!vsocket) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"socket file %s is not registered yet.\n", path);
+		return -1;
+	} else {
+		return 0;
+	}
+}
+
+int rte_vhost_driver_set_queue_num(const char *path, uint16_t queue_num)
+{
+	struct vhost_user_socket *vsocket;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		vsocket->queue_num = queue_num;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	return vsocket ? 0 : -1;
+}
+
+int rte_vhost_driver_get_queue_num(const char *path, uint16_t *queue_num)
+{
+	struct vhost_user_socket *vsocket;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		*queue_num = vsocket->queue_num;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	if (!vsocket) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"socket file %s is not registered yet.\n", path);
+		return -1;
+	} else {
+		return 0;
+	}
+}
+
 /*
  * Register a new vhost-user socket; here we could act as server
  * (the default case), or client (when RTE_VHOST_USER_CLIENT) flag
@@ -675,6 +747,11 @@  rte_vhost_driver_register(const char *path, uint64_t flags)
 		vsocket->features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
 	}
 
+	vsocket->supported_protocol_features = VHOST_USER_PROTOCOL_FEATURES;
+	vsocket->protocol_features           = VHOST_USER_PROTOCOL_FEATURES;
+
+	vsocket->queue_num = VHOST_MAX_QUEUE_PAIRS;
+
 	if ((flags & RTE_VHOST_USER_CLIENT) != 0) {
 		vsocket->reconnect = !(flags & RTE_VHOST_USER_NO_RECONNECT);
 		if (vsocket->reconnect && reconn_tid == 0) {
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index f4c7ce4..0a4d128 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -156,6 +156,18 @@  vhost_user_reset_owner(struct virtio_net *dev)
 }
 
 /*
+ * The max queue num.
+ */
+static uint16_t
+vhost_user_get_queue_num(struct virtio_net *dev)
+{
+	uint16_t queue_num = 0;
+
+	rte_vhost_driver_get_queue_num(dev->ifname, &queue_num);
+	return queue_num;
+}
+
+/*
  * The features that we support are requested.
  */
 static uint64_t
@@ -860,13 +872,17 @@  vhost_user_set_vring_enable(struct virtio_net *dev,
 	return 0;
 }
 
-static void
-vhost_user_get_protocol_features(struct virtio_net *dev,
-				 struct VhostUserMsg *msg)
+/*
+ * The protocol features that we support are requested.
+ */
+
+static uint64_t
+vhost_user_get_protocol_features(struct virtio_net *dev)
 {
-	uint64_t features, protocol_features = VHOST_USER_PROTOCOL_FEATURES;
+	uint64_t features, protocol_features;
 
 	rte_vhost_driver_get_features(dev->ifname, &features);
+	rte_vhost_driver_get_protocol_features(dev->ifname, &protocol_features);
 
 	/*
 	 * REPLY_ACK protocol feature is only mandatory for now
@@ -877,18 +893,27 @@  vhost_user_get_protocol_features(struct virtio_net *dev,
 	if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)))
 		protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK);
 
-	msg->payload.u64 = protocol_features;
-	msg->size = sizeof(msg->payload.u64);
+	return protocol_features;
 }
 
-static void
+static int
 vhost_user_set_protocol_features(struct virtio_net *dev,
 				 uint64_t protocol_features)
 {
-	if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES)
-		return;
+	uint64_t vhost_protocol_features = 0;
+
+	rte_vhost_driver_get_protocol_features(dev->ifname,
+			&vhost_protocol_features);
+	if (protocol_features & ~vhost_protocol_features) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"(%d) received invalid negotiated protocol_features.\n",
+			dev->vid);
+		return -1;
+	}
 
 	dev->protocol_features = protocol_features;
+
+	return 0;
 }
 
 static int
@@ -1252,7 +1277,8 @@  vhost_user_msg_handler(int vid, int fd)
 		break;
 
 	case VHOST_USER_GET_PROTOCOL_FEATURES:
-		vhost_user_get_protocol_features(dev, &msg);
+		msg.payload.u64 = vhost_user_get_protocol_features(dev);
+		msg.size = sizeof(msg.payload.u64);
 		send_vhost_reply(fd, &msg);
 		break;
 	case VHOST_USER_SET_PROTOCOL_FEATURES:
@@ -1312,7 +1338,7 @@  vhost_user_msg_handler(int vid, int fd)
 		break;
 
 	case VHOST_USER_GET_QUEUE_NUM:
-		msg.payload.u64 = VHOST_MAX_QUEUE_PAIRS;
+		msg.payload.u64 = vhost_user_get_queue_num(dev);
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_reply(fd, &msg);
 		break;