> -----Original Message-----
> From: Bie, Tiwei
> Sent: Tuesday, January 15, 2019 12:22 PM
> To: Stojaczyk, Dariusz <dariusz.stojac...@intel.com>
> Cc: dev@dpdk.org; Wang, Zhihong <zhihong.w...@intel.com>; Liang,
> Cunming <cunming.li...@intel.com>; Maxime Coquelin
> <maxime.coque...@redhat.com>; Harris, James R
> <james.r.har...@intel.com>; Liu, Changpeng <changpeng....@intel.com>
> Subject: Re: [PATCH v2] vhost: add external message handling callbacks to
> the public API
>
> 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.stojac...@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.
Ack
>
> > +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_*?
Ack
>
> > +
> [...]
> >
> > +/**
> > + * 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.
Ack
>
> For the rest,
> Reviewed-by: Tiwei Bie <tiwei....@intel.com>
Thanks,
D.
