From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 957555B34 for ; Tue, 13 Mar 2018 10:08:15 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 03BB7406E8BD; Tue, 13 Mar 2018 09:08:15 +0000 (UTC) Received: from [10.36.112.17] (ovpn-112-17.ams2.redhat.com [10.36.112.17]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1B0262026E03; Tue, 13 Mar 2018 09:08:11 +0000 (UTC) To: Fan Zhang , dev@dpdk.org, Tomasz Kulasek , Pawel Wodkowski Cc: jianjay.zhou@huawei.com, yliu@fridaylinux.org References: <20180227162917.35125-1-roy.fan.zhang@intel.com> <20180227162917.35125-2-roy.fan.zhang@intel.com> From: Maxime Coquelin Message-ID: <0f21227c-5b5f-a3f7-707a-60476826158e@redhat.com> Date: Tue, 13 Mar 2018 10:08:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180227162917.35125-2-roy.fan.zhang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 13 Mar 2018 09:08:15 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Tue, 13 Mar 2018 09:08:15 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: Re: [dpdk-dev] [PATCH v2 01/10] lib/librte_vhost: add vhost user private info structure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Mar 2018 09:08:15 -0000 On 02/27/2018 05:29 PM, 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 > --- > lib/librte_vhost/vhost.h | 5 ++++- > lib/librte_vhost/vhost_user.c | 13 ++++++++++++- > lib/librte_vhost/vhost_user.h | 7 +++++++ > 3 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h > index d947bc9e3..19ee3fd37 100644 > --- a/lib/librte_vhost/vhost.h > +++ b/lib/librte_vhost/vhost.h > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include I think this include isn't needed looking at the rest of the patch. > > #include > #include > @@ -241,8 +242,10 @@ struct virtio_net { > struct guest_page *guest_pages; > > int slave_req_fd; > -} __rte_cache_aligned; > > + /* Private data for different virtio device type */ > + void *private_data; > +} __rte_cache_aligned; > > #define VHOST_LOG_PAGE 4096 > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index 90ed2112e..6a90d2a96 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -1477,7 +1477,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 d4bd604b9..354615c8b 100644 > --- a/lib/librte_vhost/vhost_user.h > +++ b/lib/librte_vhost/vhost_user.h > @@ -108,6 +108,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); > I think the wording is a bit misleading, I'm fine with having a private_data pointer, but it should only be used by the external backend. Maybe what you need here is a new API to be to register a callback for the external backend to handle specific requests. Also, it might be interesting for the external backend to register callbacks for existing requests. For example .pre_vhost_user_msg_handler and .post_vhost_user_msg_handler. Doing so, the external backend could for example catch beforehand any change that could affect resources being used. Tomasz, Pawel, do you think that could help for the issue you reported? Thanks, Maxime