From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Fan Zhang <roy.fan.zhang@intel.com>,
dev@dpdk.org, Tomasz Kulasek <tomaszx.kulasek@intel.com>,
Pawel Wodkowski <pawelx.wodkowski@intel.com>
Cc: jianjay.zhou@huawei.com, yliu@fridaylinux.org
Subject: Re: [dpdk-dev] [PATCH v2 01/10] lib/librte_vhost: add vhost user private info structure
Date: Tue, 13 Mar 2018 10:08:10 +0100 [thread overview]
Message-ID: <0f21227c-5b5f-a3f7-707a-60476826158e@redhat.com> (raw)
In-Reply-To: <20180227162917.35125-2-roy.fan.zhang@intel.com>
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 <roy.fan.zhang@intel.com>
> ---
> 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 <linux/virtio_net.h>
> #include <sys/socket.h>
> #include <linux/if.h>
> +#include <stdbool.h>
I think this include isn't needed looking at the rest of the patch.
>
> #include <rte_log.h>
> #include <rte_ether.h>
> @@ -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
next prev parent reply other threads:[~2018-03-13 9:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-27 16:29 [dpdk-dev] [PATCH v2 00/10] lib/librte_vhost: introduce new vhost user crypto backend support Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 01/10] lib/librte_vhost: add vhost user private info structure Fan Zhang
2018-03-13 9:08 ` Maxime Coquelin [this message]
2018-03-21 9:10 ` Zhang, Roy Fan
2018-03-21 11:41 ` Wodkowski, PawelX
2018-03-21 13:02 ` Maxime Coquelin
2018-03-21 16:11 ` Zhang, Roy Fan
2018-03-22 8:36 ` Wodkowski, PawelX
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 02/10] lib/librte_vhost: add virtio-crypto user message structure Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 03/10] lib/librte_vhost: add session message handler Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 04/10] lib/librte_vhost: add request handler Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 05/10] lib/librte_vhost: add head file Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 06/10] lib/librte_vhost: add public function implementation Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 07/10] lib/librte_vhost: update version map Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 08/10] lib/librte_vhost: update makefile Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 09/10] examples/vhost_crypto: add vhost crypto sample application Fan Zhang
2018-02-27 16:29 ` [dpdk-dev] [PATCH v2 10/10] doc: update prog guide and sample app guid Fan Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0f21227c-5b5f-a3f7-707a-60476826158e@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=dev@dpdk.org \
--cc=jianjay.zhou@huawei.com \
--cc=pawelx.wodkowski@intel.com \
--cc=roy.fan.zhang@intel.com \
--cc=tomaszx.kulasek@intel.com \
--cc=yliu@fridaylinux.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).