[PATCH] doc: update async enqueue API usage

[PATCH] doc: update async enqueue API usage

[xuan.ding]

From: Xuan Ding <xuan.ding@intel.com>

This patch updates the correct usage for async enqueue APIs.
The rte_vhost_poll_enqueue_completed() needs to be
called in time to avoid packet loss due to full dma ring.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---
 doc/guides/prog_guide/vhost_lib.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
index cd3f6caa9a..9cdd7da154 100644
--- a/doc/guides/prog_guide/vhost_lib.rst
+++ b/doc/guides/prog_guide/vhost_lib.rst
@@ -488,3 +488,12 @@ For PA mode, page by page mapping may exceed IOMMU's max capability,
 better to use 1G guest hugepage.
 
 For UIO driver, any VFIO related error message can be ignored.
+
+Async enqueue API usage
+-----------------------
+
+In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
+called in time. Otherwise, calling rte_vhost_submit_enqueue_burst()
+all the time will cause the DMA ring to be full, which will result
+in packet loss eventually.
+
-- 
2.17.1

RE: [PATCH] doc: update async enqueue API usage

[Hu, Jiayu]

Hi Xuan,

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Friday, June 17, 2022 5:34 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>
> Subject: [PATCH] doc: update async enqueue API usage
> 
> From: Xuan Ding <xuan.ding@intel.com>
> 
> This patch updates the correct usage for async enqueue APIs.
> The rte_vhost_poll_enqueue_completed() needs to be called in time to avoid
> packet loss due to full dma ring.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  doc/guides/prog_guide/vhost_lib.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/vhost_lib.rst
> b/doc/guides/prog_guide/vhost_lib.rst
> index cd3f6caa9a..9cdd7da154 100644
> --- a/doc/guides/prog_guide/vhost_lib.rst
> +++ b/doc/guides/prog_guide/vhost_lib.rst
> @@ -488,3 +488,12 @@ For PA mode, page by page mapping may exceed
> IOMMU's max capability,  better to use 1G guest hugepage.
> 
>  For UIO driver, any VFIO related error message can be ignored.
> +
> +Async enqueue API usage
> +-----------------------

I think it's better to make it as a sub-section of "Vhost asynchronous data path".

> +
> +In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
> +called in time. Otherwise, calling rte_vhost_submit_enqueue_burst() all
> +the time will cause the DMA ring to be full, which will result in
> +packet loss eventually.

There are two main reasons of calling rte_vhost_poll_enqueue_completed() timely, IMO:
1. notify the guest of DMA copy completed packets;
2. in case of DMA ring size is smaller than vring size, it can avoid packet drop caused by
DMA ring full.

Thanks,
Jiayu


> +
> --
> 2.17.1

RE: [PATCH] doc: update async enqueue API usage

[Ding, Xuan]

Hi Jiayu,

> -----Original Message-----
> From: Hu, Jiayu <jiayu.hu@intel.com>
> Sent: Monday, June 20, 2022 9:43 AM
> To: Ding, Xuan <xuan.ding@intel.com>; maxime.coquelin@redhat.com; Xia,
> Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] doc: update async enqueue API usage
> 
> Hi Xuan,
> 
> > -----Original Message-----
> > From: Ding, Xuan <xuan.ding@intel.com>
> > Sent: Friday, June 17, 2022 5:34 PM
> > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> > <xuan.ding@intel.com>
> > Subject: [PATCH] doc: update async enqueue API usage
> >
> > From: Xuan Ding <xuan.ding@intel.com>
> >
> > This patch updates the correct usage for async enqueue APIs.
> > The rte_vhost_poll_enqueue_completed() needs to be called in time to
> > avoid packet loss due to full dma ring.
> >
> > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > ---
> >  doc/guides/prog_guide/vhost_lib.rst | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/vhost_lib.rst
> > b/doc/guides/prog_guide/vhost_lib.rst
> > index cd3f6caa9a..9cdd7da154 100644
> > --- a/doc/guides/prog_guide/vhost_lib.rst
> > +++ b/doc/guides/prog_guide/vhost_lib.rst
> > @@ -488,3 +488,12 @@ For PA mode, page by page mapping may exceed
> > IOMMU's max capability,  better to use 1G guest hugepage.
> >
> >  For UIO driver, any VFIO related error message can be ignored.
> > +
> > +Async enqueue API usage
> > +-----------------------
> 
> I think it's better to make it as a sub-section of "Vhost asynchronous data
> path".

That's a good idea, I will add this section as a sub-section accordingly.

> 
> > +
> > +In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
> > +called in time. Otherwise, calling rte_vhost_submit_enqueue_burst()
> > +all the time will cause the DMA ring to be full, which will result in
> > +packet loss eventually.
> 
> There are two main reasons of calling rte_vhost_poll_enqueue_completed()
> timely, IMO:
> 1. notify the guest of DMA copy completed packets; 2. in case of DMA ring
> size is smaller than vring size, it can avoid packet drop caused by DMA ring
> full.

Please see doc enriched in next version.

Thanks,
Xuan

> 
> Thanks,
> Jiayu
> 
> 
> > +
> > --
> > 2.17.1
> 

RE: [PATCH] doc: update async enqueue API usage

[Hu, Jiayu]

Hi Xuan,

> -----Original Message-----
> From: Hu, Jiayu
> Sent: Monday, June 20, 2022 9:43 AM
> To: Ding, Xuan <xuan.ding@intel.com>; maxime.coquelin@redhat.com; Xia,
> Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] doc: update async enqueue API usage
> 
> Hi Xuan,
> 
> > -----Original Message-----
> > From: Ding, Xuan <xuan.ding@intel.com>
> > Sent: Friday, June 17, 2022 5:34 PM
> > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> > <xuan.ding@intel.com>
> > Subject: [PATCH] doc: update async enqueue API usage
> >
> > From: Xuan Ding <xuan.ding@intel.com>
> >
> > This patch updates the correct usage for async enqueue APIs.
> > The rte_vhost_poll_enqueue_completed() needs to be called in time to
> > avoid packet loss due to full dma ring.
> >
> > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > ---
> >  doc/guides/prog_guide/vhost_lib.rst | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/vhost_lib.rst
> > b/doc/guides/prog_guide/vhost_lib.rst
> > index cd3f6caa9a..9cdd7da154 100644
> > --- a/doc/guides/prog_guide/vhost_lib.rst
> > +++ b/doc/guides/prog_guide/vhost_lib.rst
> > @@ -488,3 +488,12 @@ For PA mode, page by page mapping may exceed
> > IOMMU's max capability,  better to use 1G guest hugepage.
> >
> >  For UIO driver, any VFIO related error message can be ignored.
> > +
> > +Async enqueue API usage
> > +-----------------------
> 
> I think it's better to make it as a sub-section of "Vhost asynchronous data
> path".
> 
> > +
> > +In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
> > +called in time. Otherwise, calling rte_vhost_submit_enqueue_burst()
> > +all the time will cause the DMA ring to be full, which will result in
> > +packet loss eventually.
> 
> There are two main reasons of calling rte_vhost_poll_enqueue_completed()
> timely, IMO:
> 1. notify the guest of DMA copy completed packets; 2. in case of DMA ring
> size is smaller than vring size, it can avoid packet drop caused by DMA ring
> full.

I'd like to clarify #2 a little: not only for the case that DMA ring size < vring size.
Even if DMA ring size > vring size, it's possible dropping packets as a result of
DMA ring full.

Thanks,
Jiayu
> 
> Thanks,
> Jiayu
> 
> 
> > +
> > --
> > 2.17.1

[PATCH v2] doc: update async enqueue API usage

[xuan.ding]

From: Xuan Ding <xuan.ding@intel.com>

This patch updates the correct usage for async enqueue APIs.
The rte_vhost_poll_enqueue_completed() needs to be
called in time to notify the guest of completed packets and
avoid packet loss.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---
v2:
* refine doc and commit log
---
 doc/guides/prog_guide/vhost_lib.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
index cd3f6caa9a..628db3fc38 100644
--- a/doc/guides/prog_guide/vhost_lib.rst
+++ b/doc/guides/prog_guide/vhost_lib.rst
@@ -480,6 +480,14 @@ the same vring with their own DMA virtual channels. Besides, the number
 of DMA devices is limited. For the purpose of scaling, it's necessary to
 support sharing DMA channels among vrings.
 
+* Async enqueue API usage
+
+  In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
+  called in time to notify the guest of DMA copy completed packets.
+  Moreover, calling rte_vhost_submit_enqueue_burst() all the time but
+  not poll completed will cause the DMA ring to be full, which will
+  result in packet loss eventually.
+
 Recommended IOVA mode in async datapath
 ---------------------------------------
 
-- 
2.17.1

RE: [PATCH v2] doc: update async enqueue API usage

[Xia, Chenbo]

Hi Xuan,

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Tuesday, June 21, 2022 3:22 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>
> Subject: [PATCH v2] doc: update async enqueue API usage
> 
> From: Xuan Ding <xuan.ding@intel.com>
> 
> This patch updates the correct usage for async enqueue APIs.
> The rte_vhost_poll_enqueue_completed() needs to be
> called in time to notify the guest of completed packets and
> avoid packet loss.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
> v2:
> * refine doc and commit log
> ---
>  doc/guides/prog_guide/vhost_lib.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/vhost_lib.rst
> b/doc/guides/prog_guide/vhost_lib.rst
> index cd3f6caa9a..628db3fc38 100644
> --- a/doc/guides/prog_guide/vhost_lib.rst
> +++ b/doc/guides/prog_guide/vhost_lib.rst
> @@ -480,6 +480,14 @@ the same vring with their own DMA virtual channels.
> Besides, the number
>  of DMA devices is limited. For the purpose of scaling, it's necessary to
>  support sharing DMA channels among vrings.
> 
> +* Async enqueue API usage
> +
> +  In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
> +  called in time to notify the guest of DMA copy completed packets.
> +  Moreover, calling rte_vhost_submit_enqueue_burst() all the time but
> +  not poll completed will cause the DMA ring to be full, which will
> +  result in packet loss eventually.
> +

This subsection seems cleaner, could you also help to make 'Recommended IOVA
mode in async datapath' as a sub-section?

Another patch or not, either works for me.

Thanks,
Chenbo

>  Recommended IOVA mode in async datapath
>  ---------------------------------------
> 
> --
> 2.17.1

RE: [PATCH v2] doc: update async enqueue API usage

[Ding, Xuan]

Hi Chenbo,

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Tuesday, June 21, 2022 9:23 PM
> To: Ding, Xuan <xuan.ding@intel.com>; maxime.coquelin@redhat.com
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>
> Subject: RE: [PATCH v2] doc: update async enqueue API usage
> 
> Hi Xuan,
> 
> > -----Original Message-----
> > From: Ding, Xuan <xuan.ding@intel.com>
> > Sent: Tuesday, June 21, 2022 3:22 PM
> > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> > <xuan.ding@intel.com>
> > Subject: [PATCH v2] doc: update async enqueue API usage
> >
> > From: Xuan Ding <xuan.ding@intel.com>
> >
> > This patch updates the correct usage for async enqueue APIs.
> > The rte_vhost_poll_enqueue_completed() needs to be called in time to
> > notify the guest of completed packets and avoid packet loss.
> >
> > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > ---
> > v2:
> > * refine doc and commit log
> > ---
> >  doc/guides/prog_guide/vhost_lib.rst | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/vhost_lib.rst
> > b/doc/guides/prog_guide/vhost_lib.rst
> > index cd3f6caa9a..628db3fc38 100644
> > --- a/doc/guides/prog_guide/vhost_lib.rst
> > +++ b/doc/guides/prog_guide/vhost_lib.rst
> > @@ -480,6 +480,14 @@ the same vring with their own DMA virtual
> channels.
> > Besides, the number
> >  of DMA devices is limited. For the purpose of scaling, it's necessary
> > to  support sharing DMA channels among vrings.
> >
> > +* Async enqueue API usage
> > +
> > +  In async enqueue path, rte_vhost_poll_enqueue_completed() needs to
> > + be  called in time to notify the guest of DMA copy completed packets.
> > +  Moreover, calling rte_vhost_submit_enqueue_burst() all the time but
> > + not poll completed will cause the DMA ring to be full, which will
> > + result in packet loss eventually.
> > +
> 
> This subsection seems cleaner, could you also help to make 'Recommended
> IOVA mode in async datapath' as a sub-section?
> 
> Another patch or not, either works for me.

I will send a separate patch to move 'Recommended IOVA mode in async datapath' as a sub-section.

Thanks,
Xuan

> 
> Thanks,
> Chenbo
> 
> >  Recommended IOVA mode in async datapath
> >  ---------------------------------------
> >
> > --
> > 2.17.1

RE: [PATCH v2] doc: update async enqueue API usage

[Xia, Chenbo]

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Tuesday, June 21, 2022 3:22 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>
> Subject: [PATCH v2] doc: update async enqueue API usage
> 
> From: Xuan Ding <xuan.ding@intel.com>
> 
> This patch updates the correct usage for async enqueue APIs.
> The rte_vhost_poll_enqueue_completed() needs to be
> called in time to notify the guest of completed packets and
> avoid packet loss.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
> v2:
> * refine doc and commit log
> ---
>  doc/guides/prog_guide/vhost_lib.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/vhost_lib.rst
> b/doc/guides/prog_guide/vhost_lib.rst
> index cd3f6caa9a..628db3fc38 100644
> --- a/doc/guides/prog_guide/vhost_lib.rst
> +++ b/doc/guides/prog_guide/vhost_lib.rst
> @@ -480,6 +480,14 @@ the same vring with their own DMA virtual channels.
> Besides, the number
>  of DMA devices is limited. For the purpose of scaling, it's necessary to
>  support sharing DMA channels among vrings.
> 
> +* Async enqueue API usage
> +
> +  In async enqueue path, rte_vhost_poll_enqueue_completed() needs to be
> +  called in time to notify the guest of DMA copy completed packets.
> +  Moreover, calling rte_vhost_submit_enqueue_burst() all the time but
> +  not poll completed will cause the DMA ring to be full, which will
> +  result in packet loss eventually.
> +
>  Recommended IOVA mode in async datapath
>  ---------------------------------------
> 
> --
> 2.17.1

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

Re: [PATCH v2] doc: update async enqueue API usage

[Maxime Coquelin]

On 6/21/22 09:21, xuan.ding@intel.com wrote:
> From: Xuan Ding <xuan.ding@intel.com>
> 
> This patch updates the correct usage for async enqueue APIs.
> The rte_vhost_poll_enqueue_completed() needs to be
> called in time to notify the guest of completed packets and
> avoid packet loss.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
> v2:
> * refine doc and commit log
> ---
>   doc/guides/prog_guide/vhost_lib.rst | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime