Re: [dpdk-dev] [PATCH v2] vhost: add external message handling callbacks to the public API

[Tiwei Bie <tiwei.bie@intel.com>]

On Mon, Jan 14, 2019 at 05:28:29AM +0100, Darek Stojaczyk wrote:
> External message callbacks are used e.g. by vhost crypto
> to parse crypto-specific vhost-user messages.
> 
> We are now publishing the API to register those callbacks,
> so that other backends outside of DPDK can use them as well.
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  lib/librte_vhost/rte_vhost.h | 66 ++++++++++++++++++++++++++++++++++++
>  lib/librte_vhost/vhost.c     | 13 +++++++
>  lib/librte_vhost/vhost.h     | 54 ++---------------------------
>  3 files changed, 81 insertions(+), 52 deletions(-)
> 
> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> index d280ac420..a11f9ca04 100644
> --- a/lib/librte_vhost/rte_vhost.h
> +++ b/lib/librte_vhost/rte_vhost.h
> @@ -111,6 +111,56 @@ struct rte_vhost_vring {
>  	uint16_t		size;
>  };
>  
> +/* The possible results of a message handling function */

Better to change /* to /**, so doxygen can generate doc for it.

> +enum vh_result {
> +	/* Message handling failed */
> +	VH_RESULT_ERR   = -1,
> +	/* Message handling successful */
> +	VH_RESULT_OK    =  0,
> +	/* Message handling successful and reply prepared */
> +	VH_RESULT_REPLY =  1,
> +};

Maybe better to prefix with rte_ and RTE_?
How about rte_vhost_result and RTE_VHOST_RESULT_*?

> +
[...]
>  
> +/**
> + * Register external message handling callbacks
> + *
> + * @param vid
> + *  vhost device ID
> + * @param ops
> + *  virtio external callbacks to register
> + * @param ctx
> + *  additional context passed to the callbacks
> + * @return
> + *  0 on success, -1 on failure
> + */
> +int __rte_experimental
> +rte_vhost_extern_callback_register(int vid,
> +		struct rte_vhost_user_extern_ops const * const ops, void *ctx);

This symbol also needs to be added to the .map file
for shared library.

For the rest,
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>