From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 7866E2B82 for ; Thu, 29 Mar 2018 15:47:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Mar 2018 06:47:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,376,1517904000"; d="scan'208";a="212346705" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga005.jf.intel.com with ESMTP; 29 Mar 2018 06:47:39 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.164]) by IRSMSX154.ger.corp.intel.com ([169.254.12.249]) with mapi id 14.03.0319.002; Thu, 29 Mar 2018 14:47:38 +0100 From: "Wodkowski, PawelX" To: "Zhang, Roy Fan" , "dev@dpdk.org" CC: "maxime.coquelin@redhat.com" , "jianjay.zhou@huawei.com" , "Tan, Jianfeng" Thread-Topic: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external backend support Thread-Index: AQHTx1z+O3XG4aSdmki+NTeCov52RKPnMCfw Date: Thu, 29 Mar 2018 13:47:38 +0000 Message-ID: References: <20180326095114.11605-1-roy.fan.zhang@intel.com> <1522327975-28769-1-git-send-email-roy.fan.zhang@intel.com> <1522327975-28769-2-git-send-email-roy.fan.zhang@intel.com> In-Reply-To: <1522327975-28769-2-git-send-email-roy.fan.zhang@intel.com> Accept-Language: pl-PL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjNlY2M0ZGYtOGE1ZC00YzJmLTg0ZTYtODVhNTU4ZmU2ZDg4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjBBNldyaFV2ZjhLWVZNd24zRzBGY1wvM2VhckR5TTJ4SlwvSXNxOXRZYjNHaz0ifQ== x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external backend support 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: Thu, 29 Mar 2018 13:47:43 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang > Sent: Thursday, March 29, 2018 2:53 PM > To: dev@dpdk.org > Cc: maxime.coquelin@redhat.com; jianjay.zhou@huawei.com; Tan, Jianfeng > > Subject: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external backend > support >=20 > This patch adds external backend support to vhost library. The patch prov= ides > new APIs for the external backend to register pre and post vhost-user > message > handlers. >=20 > Signed-off-by: Fan Zhang > --- > lib/librte_vhost/rte_vhost.h | 64 > +++++++++++++++++++++++++++++++++- > lib/librte_vhost/rte_vhost_version.map | 6 ++++ > lib/librte_vhost/vhost.c | 17 ++++++++- > lib/librte_vhost/vhost.h | 8 +++-- > lib/librte_vhost/vhost_user.c | 33 +++++++++++++++++- > 5 files changed, 123 insertions(+), 5 deletions(-) >=20 > diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h > index d332069..b902c44 100644 > --- a/lib/librte_vhost/rte_vhost.h > +++ b/lib/librte_vhost/rte_vhost.h > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright(c) 2010-2017 Intel Corporation > + * Copyright(c) 2010-2018 Intel Corporation > */ >=20 > #ifndef _RTE_VHOST_H_ > @@ -88,6 +88,55 @@ struct vhost_device_ops { > }; >=20 > /** > + * function prototype for the vhost backend to handler specific vhost us= er > + * messages prior to the master message handling > + * > + * @param vid > + * vhost device id > + * @param msg > + * Message pointer. > + * @param payload > + * Message payload. No payload parameter. > + * @param require_reply > + * If the handler requires sending a reply, this varaible shall be writ= ten 1, > + * otherwise 0. > + * @param skip_master > + * If the handler requires skipping the master message handling, this > variable > + * shall be written 1, otherwise 0. > + * @return > + * 0 on success, -1 on failure > + */ > +typedef int (*rte_vhost_msg_pre_handle)(int vid, void *msg, > + uint32_t *require_reply, uint32_t *skip_master); > + > +/** > + * function prototype for the vhost backend to handler specific vhost us= er > + * messages after the master message handling is done > + * > + * @param vid > + * vhost device id > + * @param msg > + * Message pointer. > + * @param payload > + * Message payload. No payload parameter :) > + * @param require_reply > + * If the handler requires sending a reply, this varaible shall be writ= ten 1, > + * otherwise 0. > + * @return > + * 0 on success, -1 on failure > + */ > +typedef int (*rte_vhost_msg_post_handle)(int vid, void *msg, > + uint32_t *require_reply); > + What mean 'Message pointer' Is this const for us? Is this payload? Making m= sg 'void *' is not a way to go here. Those pre and post handlers need to see exactly the same structures like vhost_user.c file. Otherwise we can get into troubles when = ABI changes. Also you can easily merge pre and post handlers into one handler with one Parameter describing what phase of message processing we are now. > +/** > + * pre and post vhost user message handlers > + */ > +struct vhost_user_extern_ops { > + rte_vhost_msg_pre_handle pre_msg_handle; > + rte_vhost_msg_post_handle post_msg_handle; > +}; > + > +/** > * Convert guest physical address to host virtual address > * > * @param mem > @@ -434,6 +483,19 @@ int rte_vhost_vring_call(int vid, uint16_t vring_idx= ); > */ > uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid); >=20 > +/** > + * register external vhost backend > + * > + * @param vid > + * vhost device ID > + * @param ops > + * ops that process external vhost user messages > + * @return > + * 0 on success, -1 on failure > + */ > +int > +rte_vhost_user_register_extern_ops(int vid, struct > vhost_user_extern_ops *ops); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_vhost/rte_vhost_version.map > b/lib/librte_vhost/rte_vhost_version.map > index df01031..91bf9f0 100644 > --- a/lib/librte_vhost/rte_vhost_version.map > +++ b/lib/librte_vhost/rte_vhost_version.map > @@ -59,3 +59,9 @@ DPDK_18.02 { > rte_vhost_vring_call; >=20 > } DPDK_17.08; > + > +DPDK_18.05 { > + global: > + > + rte_vhost_user_register_extern_ops; > +} DPDK_18.02; > diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c > index a407067..80af341 100644 > --- a/lib/librte_vhost/vhost.c > +++ b/lib/librte_vhost/vhost.c > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright(c) 2010-2016 Intel Corporation > + * Copyright(c) 2010-2018 Intel Corporation > */ >=20 > #include > @@ -627,3 +627,18 @@ rte_vhost_rx_queue_count(int vid, uint16_t qid) >=20 > return *((volatile uint16_t *)&vq->avail->idx) - vq->last_avail_idx; > } > + > +int > +rte_vhost_user_register_extern_ops(int vid, struct > vhost_user_extern_ops *ops) > +{ > + struct virtio_net *dev; > + > + dev =3D get_device(vid); > + if (dev =3D=3D NULL) > + return -1; > + > + if (ops) > + rte_memcpy(&dev->extern_ops, ops, sizeof(*ops)); > + > + return 0; > +} Why we need this new "register" API? Why can't you use one of the=20 (struct vhost_device_ops).reserved[0] field to put this callback there? I think this is right time to utilize this field. Can you do something similar to=20 http://dpdk.org/ml/archives/dev/2018-March/094213.html ? > diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h > index d947bc9..2072b88 100644 > --- a/lib/librte_vhost/vhost.h > +++ b/lib/librte_vhost/vhost.h > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright(c) 2010-2014 Intel Corporation > + * Copyright(c) 2010-2018 Intel Corporation > */ >=20 > #ifndef _VHOST_NET_CDEV_H_ > @@ -241,8 +241,12 @@ struct virtio_net { > struct guest_page *guest_pages; >=20 > int slave_req_fd; > -} __rte_cache_aligned; >=20 > + /* private data for external virtio device */ > + void *extern_data; > + /* pre and post vhost user message handlers for externel backend */ > + struct vhost_user_extern_ops extern_ops; > +} __rte_cache_aligned; >=20 > #define VHOST_LOG_PAGE 4096 >=20 > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.= c > index 90ed211..ede8a5e 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright(c) 2010-2016 Intel Corporation > + * Copyright(c) 2010-2018 Intel Corporation > */ >=20 > #include > @@ -50,6 +50,8 @@ static const char > *vhost_message_str[VHOST_USER_MAX] =3D { > [VHOST_USER_NET_SET_MTU] =3D "VHOST_USER_NET_SET_MTU", > [VHOST_USER_SET_SLAVE_REQ_FD] =3D > "VHOST_USER_SET_SLAVE_REQ_FD", > [VHOST_USER_IOTLB_MSG] =3D "VHOST_USER_IOTLB_MSG", > + [VHOST_USER_CRYPTO_CREATE_SESS] =3D > "VHOST_USER_CRYPTO_CREATE_SESS", > + [VHOST_USER_CRYPTO_CLOSE_SESS] =3D > "VHOST_USER_CRYPTO_CLOSE_SESS", > }; >=20 > static uint64_t > @@ -1302,6 +1304,7 @@ vhost_user_msg_handler(int vid, int fd) > struct VhostUserMsg msg; > int ret; > int unlock_required =3D 0; > + uint32_t skip_master =3D 0; >=20 > dev =3D get_device(vid); > if (dev =3D=3D NULL) > @@ -1379,6 +1382,21 @@ vhost_user_msg_handler(int vid, int fd) >=20 > } >=20 > + if (dev->extern_ops.pre_msg_handle) { > + uint32_t need_reply; > + > + ret =3D (*dev->extern_ops.pre_msg_handle)(dev->vid, > + (void *)&msg, &need_reply, &skip_master); > + if (ret < 0) > + goto skip_to_reply; > + > + if (need_reply) > + send_vhost_reply(fd, &msg); > + } > + > + if (skip_master) > + goto skip_to_post_handle; This can be moved inside above if () { }=20 > + > switch (msg.request.master) { > case VHOST_USER_GET_FEATURES: > msg.payload.u64 =3D vhost_user_get_features(dev); > @@ -1479,9 +1497,22 @@ vhost_user_msg_handler(int vid, int fd) > default: > ret =3D -1; > break; > + } > + > +skip_to_post_handle: > + if (dev->extern_ops.post_msg_handle) { > + uint32_t need_reply; > + > + ret =3D (*dev->extern_ops.post_msg_handle)( > + dev->vid, (void *)&msg, &need_reply); > + if (ret < 0) > + goto skip_to_reply; >=20 > + if (need_reply) > + send_vhost_reply(fd, &msg); > } >=20 > +skip_to_reply: > if (unlock_required) > vhost_user_unlock_all_queue_pairs(dev); >=20 > -- > 2.7.4 Overall, I think, this direction where we need to go. Pawel