[dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external backend support

[Wodkowski, PawelX]

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Tuesday, April 3, 2018 4:43 PM
> To: Maxime Coquelin <maxime.coquelin at redhat.com>; Zhang, Roy Fan
> <roy.fan.zhang at intel.com>; Wodkowski, PawelX
> <pawelx.wodkowski at intel.com>; dev at dpdk.org
> Cc: jianjay.zhou at huawei.com
> Subject: Re: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external
> backend support
> 
> 
> 
> On 4/3/2018 9:44 PM, Maxime Coquelin wrote:
> > 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.zhang at intel.com>; dev at dpdk.org
> >>> Cc: maxime.coquelin at redhat.com; jianjay.zhou at huawei.com; Tan,
> Jianfeng
> >>> <jianfeng.tan at intel.com>
> >>> Subject: RE: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external
> >>> backend support
> >>>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Fan Zhang
> >>>> Sent: Thursday, March 29, 2018 2:53 PM
> >>>> To: dev at dpdk.org
> >>>> Cc: maxime.coquelin at redhat.com; jianjay.zhou at huawei.com; Tan,
> >>> Jianfeng
> >>>> <jianfeng.tan at 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.zhang at 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?
> 
> +1. This can avoid blocking this patch set, and give more time for
> discussing new APIs and external structs.
> 
> Thanks,
> Jianfeng
> 
> >
> > Thanks,
> > Maxime

Fine for me too.
Pawel