From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yliu@fridaylinux.org>
Subject: Re: [PATCH 02/12] lib/librte_vhost: add vhost_user
 private info structure
Date: Thu, 18 Jan 2018 22:36:10 +0800
Message-ID: <20180118143610.GL29540@yliu-mob>
References: <20171127200115.31049-1-roy.fan.zhang@intel.com>
 <20171127200115.31049-3-roy.fan.zhang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, maxime.coquelin@redhat.com, tiwei.bie@intel.com
To: Fan Zhang <roy.fan.zhang@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 8538D1B316
 for <dev@dpdk.org>; Thu, 18 Jan 2018 15:36:15 +0100 (CET)
Content-Disposition: inline
In-Reply-To: <20171127200115.31049-3-roy.fan.zhang@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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