Hi Maxime, No problem. I will work on that. Pawel, Jianfeng, if you guys have other concerns or suggestions, please give me a shout.
Thanks a lot guys, for the review and help! Regards, Fan > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Tuesday, April 3, 2018 2:45 PM > To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; Wodkowski, PawelX > <pawelx.wodkow...@intel.com>; dev@dpdk.org > Cc: jianjay.z...@huawei.com; Tan, Jianfeng <jianfeng....@intel.com> > Subject: Re: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external > backend support > > Hi Pawel, Fan, > > On 04/01/2018 09:53 PM, Zhang, Roy Fan wrote: > > Hi Pawel, > > > >> -----Original Message----- > >> From: Wodkowski, PawelX > >> Sent: Thursday, March 29, 2018 2:48 PM > >> To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; dev@dpdk.org > >> Cc: maxime.coque...@redhat.com; jianjay.z...@huawei.com; Tan, > >> Jianfeng <jianfeng....@intel.com> > >> Subject: RE: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external > >> backend support > >> > >>> -----Original Message----- > >>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Fan Zhang > >>> Sent: Thursday, March 29, 2018 2:53 PM > >>> To: dev@dpdk.org > >>> Cc: maxime.coque...@redhat.com; jianjay.z...@huawei.com; Tan, > >> Jianfeng > >>> <jianfeng....@intel.com> > >>> Subject: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external > >>> backend support > >>> > >>> This patch adds external backend support to vhost library. The patch > >>> provides new APIs for the external backend to register pre and post > >>> vhost-user message handlers. > >>> > >>> Signed-off-by: Fan Zhang <roy.fan.zh...@intel.com> > >>> --- > >>> 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(-) > >>> > >>> 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 @@ > > <snip/> > > > > >>> + * @param require_reply > >>> + * If the handler requires sending a reply, this varaible shall be > >>> +written 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 msg '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. > > > > It is the pointer to the vhost_user message. It cannot be const as the > > backend may change the payload. > > > >> > >> Also you can easily merge pre and post handlers into one handler with > >> one Parameter describing what phase of message processing we are now. > >> > > > > No I don't think so. To do so it will be quite unclear in the future > > as we are using one function to do two totally different things. > > Time is running out for v18.05 integration deadline (April 6th), and we > haven't > reached a consensus. > > Except this API point, I think vhost-crypto is at the right level. > Since vhost-crypto lives in librte_vhost, I propose Fan cooks an intermediate > solution that does not need API change. > > Doing this, we postpone the API change to v18.08, so we have time to discuss > what the right API should be. Once agreed, vhost-crypto moves to the new > API. > > Pawel, Jianfeng, Fan, is it fine for you? > > Thanks, > Maxime
