From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id EBFDF4C9D for ; Tue, 19 Mar 2019 11:15:29 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4C1A03024563; Tue, 19 Mar 2019 10:15:29 +0000 (UTC) Received: from [10.36.112.48] (ovpn-112-48.ams2.redhat.com [10.36.112.48]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CA03E1001DD9; Tue, 19 Mar 2019 10:15:27 +0000 (UTC) To: Ilya Maximets , dev@dpdk.org, changpeng.liu@intel.com, tiwei.bie@intel.com, dariusz.stojaczyk@intel.com References: <20190313155504.15087-1-maxime.coquelin@redhat.com> <20190313155504.15087-3-maxime.coquelin@redhat.com> <03ff92ec-7068-8312-669d-53519878c6cd@samsung.com> From: Maxime Coquelin Message-ID: Date: Tue, 19 Mar 2019 11:15:24 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <03ff92ec-7068-8312-669d-53519878c6cd@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Tue, 19 Mar 2019 10:15:29 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 2/2] vhost: support requests only handled by external backend 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: Tue, 19 Mar 2019 10:15:30 -0000 Hi Ilya, On 3/13/19 5:09 PM, Ilya Maximets wrote: > On 13.03.2019 18:55, Maxime Coquelin wrote: >> External backends may have specific requests to handle, and so >> we don't want the vhost-user lib to handle these requests as >> errors. >> >> This patch also changes the experimental API by introducing >> RTE_VHOST_MSG_RESULT_NOT_HANDLED so that vhost-user lib >> can report an error if a message is handled neither by >> the vhost-user library nor by the external backend. >> >> The logic changes a bit so that if the callback returns >> with ERR, OK or REPLY, it is considered the message >> is handled by the external backend so it won't be >> handled by the vhost-user library. >> It is still possible for an external backend to listen >> to requests that have to be handled by the vhost-user >> library like SET_MEM_TABLE, but the callback have to >> return NOT_HANDLED in that case. >> >> Vhost-crypto backend is ialso adapted to this API change. >> >> Suggested-by: Ilya Maximets >> Signed-off-by: Maxime Coquelin >> Tested-by: Darek Stojaczyk >> --- >> lib/librte_vhost/rte_vhost.h | 33 ++++--------- >> lib/librte_vhost/vhost_crypto.c | 10 +++- >> lib/librte_vhost/vhost_user.c | 82 +++++++++++++++++++++------------ >> 3 files changed, 69 insertions(+), 56 deletions(-) >> >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h >> index c9c392975..8ab4ff299 100644 >> --- a/lib/librte_vhost/rte_vhost.h >> +++ b/lib/librte_vhost/rte_vhost.h >> @@ -121,6 +121,8 @@ enum rte_vhost_msg_result { >> RTE_VHOST_MSG_RESULT_OK = 0, >> /* Message handling successful and reply prepared */ >> RTE_VHOST_MSG_RESULT_REPLY = 1, >> + /* Message not handled */ >> + RTE_VHOST_MSG_RESULT_NOT_HANDLED, >> }; >> >> /** >> @@ -131,37 +133,20 @@ enum rte_vhost_msg_result { > /** > * Function prototype for the vhost backend to handler specific vhost user > * messages prior to the master message handling > > Now you have definition of pre handler for a common handler type. > > It should be updated. Probably just cropped a bit (and s/handler/handle/): > > /** > * Function prototype for the vhost backend to handle specific vhost user > * messages. Indeed, I missed to fix that. >> * vhost device id >> * @param msg >> * Message pointer. >> - * @param skip_master >> - * If the handler requires skipping the master message handling, this variable >> - * shall be written 1, otherwise 0. >> * @return >> - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, >> - * VH_RESULT_ERR on failure >> + * RTE_VHOST_MSG_RESULT_OK on success, >> + * RTE_VHOST_MSG_RESULT_REPLY on success with reply, >> + * RTE_VHOST_MSG_RESULT_ERR on failure, >> + * RTE_VHOST_MSG_RESULT_NOT_HANDLED if message was not handled. >> */ >> -typedef enum rte_vhost_msg_result (*rte_vhost_msg_pre_handle)(int vid, >> - void *msg, uint32_t *skip_master); >> - >> -/** >> - * Function prototype for the vhost backend to handler specific vhost user >> - * messages after the master message handling is done >> - * >> - * @param vid >> - * vhost device id >> - * @param msg >> - * Message pointer. >> - * @return >> - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, >> - * VH_RESULT_ERR on failure >> - */ >> -typedef enum rte_vhost_msg_result (*rte_vhost_msg_post_handle)(int vid, >> - void *msg); >> +typedef enum rte_vhost_msg_result (*rte_vhost_msg_handle)(int vid, void *msg); >> >> /** >> * Optional vhost user message handlers. >> */ >> struct rte_vhost_user_extern_ops { >> - rte_vhost_msg_pre_handle pre_msg_handle; >> - rte_vhost_msg_post_handle post_msg_handle; > What about some comments here? > > /* Called prior to the master message handling. */ >> + rte_vhost_msg_handle pre_msg_handle; > /* Called after the master message handling. */ >> + rte_vhost_msg_handle post_msg_handle; >> }; >> That's good suggestions, I'll post a v3 with these changes. Thanks, Maxime From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 5B825A00E6 for ; Tue, 19 Mar 2019 11:15:32 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2CC2D4C9D; Tue, 19 Mar 2019 11:15:32 +0100 (CET) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id EBFDF4C9D for ; Tue, 19 Mar 2019 11:15:29 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4C1A03024563; Tue, 19 Mar 2019 10:15:29 +0000 (UTC) Received: from [10.36.112.48] (ovpn-112-48.ams2.redhat.com [10.36.112.48]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CA03E1001DD9; Tue, 19 Mar 2019 10:15:27 +0000 (UTC) To: Ilya Maximets , dev@dpdk.org, changpeng.liu@intel.com, tiwei.bie@intel.com, dariusz.stojaczyk@intel.com References: <20190313155504.15087-1-maxime.coquelin@redhat.com> <20190313155504.15087-3-maxime.coquelin@redhat.com> <03ff92ec-7068-8312-669d-53519878c6cd@samsung.com> From: Maxime Coquelin Message-ID: Date: Tue, 19 Mar 2019 11:15:24 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <03ff92ec-7068-8312-669d-53519878c6cd@samsung.com> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Tue, 19 Mar 2019 10:15:29 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 2/2] vhost: support requests only handled by external backend 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190319101524.Z-hiV6inrMlEVjoRViXx1-q3x8MSbwJobB7VvT-D3ZM@z> Hi Ilya, On 3/13/19 5:09 PM, Ilya Maximets wrote: > On 13.03.2019 18:55, Maxime Coquelin wrote: >> External backends may have specific requests to handle, and so >> we don't want the vhost-user lib to handle these requests as >> errors. >> >> This patch also changes the experimental API by introducing >> RTE_VHOST_MSG_RESULT_NOT_HANDLED so that vhost-user lib >> can report an error if a message is handled neither by >> the vhost-user library nor by the external backend. >> >> The logic changes a bit so that if the callback returns >> with ERR, OK or REPLY, it is considered the message >> is handled by the external backend so it won't be >> handled by the vhost-user library. >> It is still possible for an external backend to listen >> to requests that have to be handled by the vhost-user >> library like SET_MEM_TABLE, but the callback have to >> return NOT_HANDLED in that case. >> >> Vhost-crypto backend is ialso adapted to this API change. >> >> Suggested-by: Ilya Maximets >> Signed-off-by: Maxime Coquelin >> Tested-by: Darek Stojaczyk >> --- >> lib/librte_vhost/rte_vhost.h | 33 ++++--------- >> lib/librte_vhost/vhost_crypto.c | 10 +++- >> lib/librte_vhost/vhost_user.c | 82 +++++++++++++++++++++------------ >> 3 files changed, 69 insertions(+), 56 deletions(-) >> >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h >> index c9c392975..8ab4ff299 100644 >> --- a/lib/librte_vhost/rte_vhost.h >> +++ b/lib/librte_vhost/rte_vhost.h >> @@ -121,6 +121,8 @@ enum rte_vhost_msg_result { >> RTE_VHOST_MSG_RESULT_OK = 0, >> /* Message handling successful and reply prepared */ >> RTE_VHOST_MSG_RESULT_REPLY = 1, >> + /* Message not handled */ >> + RTE_VHOST_MSG_RESULT_NOT_HANDLED, >> }; >> >> /** >> @@ -131,37 +133,20 @@ enum rte_vhost_msg_result { > /** > * Function prototype for the vhost backend to handler specific vhost user > * messages prior to the master message handling > > Now you have definition of pre handler for a common handler type. > > It should be updated. Probably just cropped a bit (and s/handler/handle/): > > /** > * Function prototype for the vhost backend to handle specific vhost user > * messages. Indeed, I missed to fix that. >> * vhost device id >> * @param msg >> * Message pointer. >> - * @param skip_master >> - * If the handler requires skipping the master message handling, this variable >> - * shall be written 1, otherwise 0. >> * @return >> - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, >> - * VH_RESULT_ERR on failure >> + * RTE_VHOST_MSG_RESULT_OK on success, >> + * RTE_VHOST_MSG_RESULT_REPLY on success with reply, >> + * RTE_VHOST_MSG_RESULT_ERR on failure, >> + * RTE_VHOST_MSG_RESULT_NOT_HANDLED if message was not handled. >> */ >> -typedef enum rte_vhost_msg_result (*rte_vhost_msg_pre_handle)(int vid, >> - void *msg, uint32_t *skip_master); >> - >> -/** >> - * Function prototype for the vhost backend to handler specific vhost user >> - * messages after the master message handling is done >> - * >> - * @param vid >> - * vhost device id >> - * @param msg >> - * Message pointer. >> - * @return >> - * VH_RESULT_OK on success, VH_RESULT_REPLY on success with reply, >> - * VH_RESULT_ERR on failure >> - */ >> -typedef enum rte_vhost_msg_result (*rte_vhost_msg_post_handle)(int vid, >> - void *msg); >> +typedef enum rte_vhost_msg_result (*rte_vhost_msg_handle)(int vid, void *msg); >> >> /** >> * Optional vhost user message handlers. >> */ >> struct rte_vhost_user_extern_ops { >> - rte_vhost_msg_pre_handle pre_msg_handle; >> - rte_vhost_msg_post_handle post_msg_handle; > What about some comments here? > > /* Called prior to the master message handling. */ >> + rte_vhost_msg_handle pre_msg_handle; > /* Called after the master message handling. */ >> + rte_vhost_msg_handle post_msg_handle; >> }; >> That's good suggestions, I'll post a v3 with these changes. Thanks, Maxime