Re: [dpdk-dev] [PATCH v1] vhost: fix async callback return type define

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 7/23/20 7:39 AM, patrick.fu@intel.com wrote:
> From: Patrick Fu <patrick.fu@intel.com>
> 
> The async copy device callbacks are used by async APIs to transfer data
> and check completion status. Async APIs return the number of packets
> successfully processed to the caller applications and no error (negative)
> value is allowed for API return value. Thus, negative return values
> from async device callbacks don't have meaningful usage, while adding
> overhead in checking the return value validity. This patch change the
> callback return values from "int" to "uint32_t" to get aligned with
> async API definition.
> 
> Fixes: 78639d54563a ("vhost: introduce async enqueue registration API")
> 
> Signed-off-by: Patrick Fu <patrick.fu@intel.com>
> ---
>  lib/librte_vhost/rte_vhost_async.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_vhost/rte_vhost_async.h b/lib/librte_vhost/rte_vhost_async.h
> index c8ad8dbc7..66d258abe 100644
> --- a/lib/librte_vhost/rte_vhost_async.h
> +++ b/lib/librte_vhost/rte_vhost_async.h
> @@ -59,9 +59,9 @@ struct rte_vhost_async_channel_ops {
>  	 * @param count
>  	 *  number of elements in the "descs" array
>  	 * @return
> -	 *  -1 on failure, number of descs processed on success
> +	 *  number of descs processed on success

let's remove on success as in case of failure it will still be the
number of descriptors process, i.e. 0.

>  	 */
> -	int (*transfer_data)(int vid, uint16_t queue_id,
> +	uint32_t (*transfer_data)(int vid, uint16_t queue_id,
>  		struct rte_vhost_async_desc *descs,
>  		struct rte_vhost_async_status *opaque_data,
>  		uint16_t count);
> @@ -70,15 +70,15 @@ struct rte_vhost_async_channel_ops {
>  	 * @param vid
>  	 *  id of vhost device to check copy completion
>  	 * @param queue_id
> -	 *  queue id to check copyp completion
> +	 *  queue id to check copy completion
>  	 * @param opaque_data
>  	 *  buffer to receive the opaque data pair from DMA engine
>  	 * @param max_packets
>  	 *  max number of packets could be completed
>  	 * @return
> -	 *  -1 on failure, number of iov segments completed on success
> +	 *  number of iov segments completed on success
Ditto

>  	 */
> -	int (*check_completed_copies)(int vid, uint16_t queue_id,
> +	uint32_t (*check_completed_copies)(int vid, uint16_t queue_id,
>  		struct rte_vhost_async_status *opaque_data,
>  		uint16_t max_packets);
>  };
> 

Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

I can fixup the comments while applying if you agree with it.

Thanks,
Maxime