DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Ilya Maximets <i.maximets@samsung.com>,
	dev@dpdk.org, changpeng.liu@intel.com, tiwei.bie@intel.com,
	dariusz.stojaczyk@intel.com
Subject: Re: [dpdk-dev] [PATCH v2 2/2] vhost: support requests only handled by external backend
Date: Tue, 19 Mar 2019 11:15:24 +0100	[thread overview]
Message-ID: <ba7fe2a4-bbca-847d-7daa-f3ae676452ff@redhat.com> (raw)
Message-ID: <20190319101524.Z-hiV6inrMlEVjoRViXx1-q3x8MSbwJobB7VvT-D3ZM@z> (raw)
In-Reply-To: <03ff92ec-7068-8312-669d-53519878c6cd@samsung.com>

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<i.maximets@samsung.com>
>> Signed-off-by: Maxime Coquelin<maxime.coquelin@redhat.com>
>> Tested-by: Darek Stojaczyk<dariusz.stojaczyk@intel.com>
>> ---
>>   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

  reply	other threads:[~2019-03-19 10:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13 15:55 [dpdk-dev] [PATCH v2 0/2] vhost: Support external backend only vhost-user requests Maxime Coquelin
2019-03-13 15:55 ` [dpdk-dev] [PATCH v2 1/2] vhost: add API to set protocol features flags Maxime Coquelin
2019-03-13 15:55 ` [dpdk-dev] [PATCH v2 2/2] vhost: support requests only handled by external backend Maxime Coquelin
2019-03-13 16:09   ` Ilya Maximets
2019-03-19 10:15     ` Maxime Coquelin [this message]
2019-03-19 10:15       ` Maxime Coquelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba7fe2a4-bbca-847d-7daa-f3ae676452ff@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=changpeng.liu@intel.com \
    --cc=dariusz.stojaczyk@intel.com \
    --cc=dev@dpdk.org \
    --cc=i.maximets@samsung.com \
    --cc=tiwei.bie@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).