DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] vhost: fix async callback return type define
@ 2020-07-23  5:39 patrick.fu
  2020-07-27 12:52 ` Xia, Chenbo
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: patrick.fu @ 2020-07-23  5:39 UTC (permalink / raw)
  To: dev, maxime.coquelin, chenbo.xia; +Cc: Patrick Fu

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
 	 */
-	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
 	 */
-	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);
 };
-- 
2.18.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH v1] vhost: fix async callback return type define
  2020-07-23  5:39 [dpdk-dev] [PATCH v1] vhost: fix async callback return type define patrick.fu
@ 2020-07-27 12:52 ` Xia, Chenbo
  2020-07-28  9:10 ` Maxime Coquelin
  2020-07-28 15:26 ` Maxime Coquelin
  2 siblings, 0 replies; 5+ messages in thread
From: Xia, Chenbo @ 2020-07-27 12:52 UTC (permalink / raw)
  To: Fu, Patrick, dev, maxime.coquelin


> -----Original Message-----
> From: Fu, Patrick <patrick.fu@intel.com>
> Sent: Thursday, July 23, 2020 1:39 PM
> To: dev@dpdk.org; maxime.coquelin@redhat.com; Xia, Chenbo
> <chenbo.xia@intel.com>
> Cc: Fu, Patrick <patrick.fu@intel.com>
> Subject: [PATCH v1] vhost: fix async callback return type define
> 
> 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
>  	 */
> -	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
>  	 */
> -	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);
>  };
> --
> 2.18.4

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH v1] vhost: fix async callback return type define
  2020-07-23  5:39 [dpdk-dev] [PATCH v1] vhost: fix async callback return type define patrick.fu
  2020-07-27 12:52 ` Xia, Chenbo
@ 2020-07-28  9:10 ` Maxime Coquelin
  2020-07-28 10:15   ` Fu, Patrick
  2020-07-28 15:26 ` Maxime Coquelin
  2 siblings, 1 reply; 5+ messages in thread
From: Maxime Coquelin @ 2020-07-28  9:10 UTC (permalink / raw)
  To: patrick.fu, dev, chenbo.xia



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH v1] vhost: fix async callback return type define
  2020-07-28  9:10 ` Maxime Coquelin
@ 2020-07-28 10:15   ` Fu, Patrick
  0 siblings, 0 replies; 5+ messages in thread
From: Fu, Patrick @ 2020-07-28 10:15 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, July 28, 2020 5:10 PM
> To: Fu, Patrick <patrick.fu@intel.com>; dev@dpdk.org; Xia, Chenbo
> <chenbo.xia@intel.com>
> Subject: Re: [PATCH v1] vhost: fix async callback return type define
> 
> 
> 
> On 7/23/20 7:39 AM, patrick.fu@intel.com wrote:
> > From: Patrick Fu <patrick.fu@intel.com>
> >
> >  	 * @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.
> 

Yes, I think removing "on success" is correct there.

Thanks,

Patrick


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH v1] vhost: fix async callback return type define
  2020-07-23  5:39 [dpdk-dev] [PATCH v1] vhost: fix async callback return type define patrick.fu
  2020-07-27 12:52 ` Xia, Chenbo
  2020-07-28  9:10 ` Maxime Coquelin
@ 2020-07-28 15:26 ` Maxime Coquelin
  2 siblings, 0 replies; 5+ messages in thread
From: Maxime Coquelin @ 2020-07-28 15:26 UTC (permalink / raw)
  To: patrick.fu, dev, chenbo.xia



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(-)

Applied to dpdk-next-virtio/master with agreed fixup.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-28 15:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23  5:39 [dpdk-dev] [PATCH v1] vhost: fix async callback return type define patrick.fu
2020-07-27 12:52 ` Xia, Chenbo
2020-07-28  9:10 ` Maxime Coquelin
2020-07-28 10:15   ` Fu, Patrick
2020-07-28 15:26 ` Maxime Coquelin

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).