[dpdk-dev,02/12] lib/librte_vhost: add vhost_user private info structure

Message ID 20171127200115.31049-3-roy.fan.zhang@intel.com
State Superseded, archived
Delegated to: Yuanhan Liu
Headers show

Checks

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

Commit Message

Fan Zhang Nov. 27, 2017, 8:01 p.m.
This patch adds a vhost_user_dev_priv structure and a vhost_user
message handler function prototype to vhost_user. This allows
different types of devices to add private information and their
device-specific vhost-user message function handlers to
virtio_net structure. The change to vhost_user_msg_handler is
also added to call the device-specific message handler.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 lib/librte_vhost/vhost_user.c | 13 ++++++++++++-
 lib/librte_vhost/vhost_user.h |  9 ++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Yuanhan Liu Jan. 18, 2018, 2:36 p.m. | #1
On Mon, Nov 27, 2017 at 08:01:05PM +0000, Fan Zhang wrote:
> This patch adds a vhost_user_dev_priv structure and a vhost_user
> message handler function prototype to vhost_user. This allows
> different types of devices to add private information and their
> device-specific vhost-user message function handlers to
> virtio_net structure. The change to vhost_user_msg_handler is
> also added to call the device-specific message handler.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  lib/librte_vhost/vhost_user.c | 13 ++++++++++++-
>  lib/librte_vhost/vhost_user.h |  9 ++++++++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index f4c7ce4..c5ae85f 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1337,7 +1337,18 @@ vhost_user_msg_handler(int vid, int fd)
>  		break;
>  
>  	default:

Hi Fan,

Firstly, apologize for the very late review!

> -		ret = -1;
> +		if (!dev->private_data)
> +			ret = -1;
> +		else {
> +			struct vhost_user_dev_priv *priv = dev->private_data;
> +
> +			if (!priv->vhost_user_msg_handler)
> +				ret = -1;
> +			else {
> +				ret = (*priv->vhost_user_msg_handler)(dev,
> +						&msg, fd);

I think this is more complex than necessary. You could just add msg_handler
in the virtio_net struct and do:

		if (dev->msg_handler)
			ret = dev->msg_handler(dev, &msg, fd);
		else
			ret = -1;

> +			}
> +		}
>  		break;
>  
>  	}
> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> index 76d9fe2..990b40a 100644
> --- a/lib/librte_vhost/vhost_user.h
> +++ b/lib/librte_vhost/vhost_user.h
> @@ -81,7 +81,7 @@ typedef enum VhostUserRequest {
>  	VHOST_USER_NET_SET_MTU = 20,
>  	VHOST_USER_SET_SLAVE_REQ_FD = 21,
>  	VHOST_USER_IOTLB_MSG = 22,
> -	VHOST_USER_MAX
> +	VHOST_USER_MAX = 25

That's weird, we should list all VHOST_USER messages here.

	--yliu

>  } VhostUserRequest;
>  
>  typedef enum VhostUserSlaveRequest {
> @@ -137,6 +137,13 @@ typedef struct VhostUserMsg {
>  /* The version of the protocol we support */
>  #define VHOST_USER_VERSION    0x1
>  
> +typedef int (*msg_handler)(struct virtio_net *dev, struct VhostUserMsg *msg,
> +		int fd);
> +
> +struct vhost_user_dev_priv {
> +	msg_handler vhost_user_msg_handler;
> +	char data[0];
> +};
>  
>  /* vhost_user.c */
>  int vhost_user_msg_handler(int vid, int fd);
> -- 
> 2.9.5

Patch

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index f4c7ce4..c5ae85f 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1337,7 +1337,18 @@  vhost_user_msg_handler(int vid, int fd)
 		break;
 
 	default:
-		ret = -1;
+		if (!dev->private_data)
+			ret = -1;
+		else {
+			struct vhost_user_dev_priv *priv = dev->private_data;
+
+			if (!priv->vhost_user_msg_handler)
+				ret = -1;
+			else {
+				ret = (*priv->vhost_user_msg_handler)(dev,
+						&msg, fd);
+			}
+		}
 		break;
 
 	}
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 76d9fe2..990b40a 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -81,7 +81,7 @@  typedef enum VhostUserRequest {
 	VHOST_USER_NET_SET_MTU = 20,
 	VHOST_USER_SET_SLAVE_REQ_FD = 21,
 	VHOST_USER_IOTLB_MSG = 22,
-	VHOST_USER_MAX
+	VHOST_USER_MAX = 25
 } VhostUserRequest;
 
 typedef enum VhostUserSlaveRequest {
@@ -137,6 +137,13 @@  typedef struct VhostUserMsg {
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION    0x1
 
+typedef int (*msg_handler)(struct virtio_net *dev, struct VhostUserMsg *msg,
+		int fd);
+
+struct vhost_user_dev_priv {
+	msg_handler vhost_user_msg_handler;
+	char data[0];
+};
 
 /* vhost_user.c */
 int vhost_user_msg_handler(int vid, int fd);