From mboxrd@z Thu Jan  1 00:00:00 1970
From: Maxime Coquelin <maxime.coquelin@redhat.com>
Subject: Re: [PATCH v4 1/8] lib/librte_vhost: add external
 backend support
Date: Tue, 3 Apr 2018 15:44:40 +0200
Message-ID: <d1eb3a59-6da7-8f6c-9209-0228293f6198@redhat.com>
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>
 <F6F2A6264E145F47A18AB6DF8E87425D701190BA@IRSMSX102.ger.corp.intel.com>
 <9F7182E3F746AB4EA17801C148F3C60433107799@IRSMSX101.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: "jianjay.zhou@huawei.com" <jianjay.zhou@huawei.com>,
 "Tan, Jianfeng" <jianfeng.tan@intel.com>
To: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>,
 "Wodkowski, PawelX" <pawelx.wodkowski@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>
Return-path: <dev-bounces@dpdk.org>
Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73])
 by dpdk.org (Postfix) with ESMTP id 8F30C1B72F
 for <dev@dpdk.org>; Tue,  3 Apr 2018 15:44:44 +0200 (CEST)
In-Reply-To: <9F7182E3F746AB4EA17801C148F3C60433107799@IRSMSX101.ger.corp.intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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@intel.com>; dev@dpdk.org
>> Cc: maxime.coquelin@redhat.com; jianjay.zhou@huawei.com; Tan, Jianfeng
>> <jianfeng.tan@intel.com>
>> Subject: RE: [dpdk-dev] [PATCH v4 1/8] lib/librte_vhost: add external
>> backend support
>>
>>> -----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
>>> <jianfeng.tan@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@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