[v3,2/8] dmadev: add burst capacity API
Checks
Commit Message
From: Kevin Laatz <kevin.laatz@intel.com>
Add a burst capacity check API to the dmadev library. This API is useful to
applications which need to how many descriptors can be enqueued in the
current batch. For example, it could be used to determine whether all
segments of a multi-segment packet can be enqueued in the same batch or not
(to avoid half-offload of the packet).
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
lib/dmadev/rte_dmadev.c | 11 +++++++++++
lib/dmadev/rte_dmadev.h | 19 +++++++++++++++++++
lib/dmadev/rte_dmadev_core.h | 5 +++++
lib/dmadev/version.map | 1 +
4 files changed, 36 insertions(+)
Comments
> Subject: [PATCH v3 2/8] dmadev: add burst capacity API
>
> From: Kevin Laatz <kevin.laatz@intel.com>
>
> Add a burst capacity check API to the dmadev library. This API is useful to
> applications which need to how many descriptors can be enqueued in the
> current batch. For example, it could be used to determine whether all
> segments of a multi-segment packet can be enqueued in the same batch or
> not
> (to avoid half-offload of the packet).
>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
Reviewed-by: Conor Walsh <conor.walsh@intel.com>
<snip>
On Tue, 7 Sep 2021, 10:25 pm Bruce Richardson, <bruce.richardson@intel.com>
wrote:
> From: Kevin Laatz <kevin.laatz@intel.com>
>
> Add a burst capacity check API to the dmadev library. This API is useful to
> applications which need to how many descriptors can be enqueued in the
> current batch. For example, it could be used to determine whether all
> segments of a multi-segment packet can be enqueued in the same batch or not
> (to avoid half-offload of the packet).
>
#Could you share more details on the use case with vhost?
# Are they planning to use this in fast path if so it need to move as fast
path function pointer?
# Assume the use case needs N rte_dma_copy to complete a logical copy at
vhost level. Is the any issue in half-offload, meaning when N th one
successfully completed then only the logical copy is completed. Right?
# There is already nb_desc with which a dma_queue is configured. So if the
application does its accounting properly, it knows how many desc it has
used up and how many completions it has processed.
Would like to understand more details on this API usage.
Sorry for the format issue, sending from mobile.
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
> lib/dmadev/rte_dmadev.c | 11 +++++++++++
> lib/dmadev/rte_dmadev.h | 19 +++++++++++++++++++
> lib/dmadev/rte_dmadev_core.h | 5 +++++
> lib/dmadev/version.map | 1 +
> 4 files changed, 36 insertions(+)
>
> diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
> index ab45928efb..6494871f05 100644
> --- a/lib/dmadev/rte_dmadev.c
> +++ b/lib/dmadev/rte_dmadev.c
> @@ -573,6 +573,17 @@ dmadev_dump_capability(FILE *f, uint64_t dev_capa)
> fprintf(f, "\n");
> }
>
> +int
> +rte_dmadev_burst_capacity(uint16_t dev_id, uint16_t vchan)
> +{
> + const struct rte_dmadev *dev = &rte_dmadevices[dev_id];
> +
> + RTE_DMADEV_VALID_DEV_ID_OR_ERR_RET(dev_id, -EINVAL);
> +
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->burst_capacity, -ENOTSUP);
> + return (*dev->dev_ops->burst_capacity)(dev, vchan);
> +}
> +
> int
> rte_dmadev_dump(uint16_t dev_id, FILE *f)
> {
> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
> index 39d73872c8..8b84914810 100644
> --- a/lib/dmadev/rte_dmadev.h
> +++ b/lib/dmadev/rte_dmadev.h
> @@ -673,6 +673,25 @@ __rte_experimental
> int
> rte_dmadev_vchan_status(uint16_t dev_id, uint16_t vchan, enum
> rte_dmadev_vchan_status *status);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Check remaining capacity in descriptor ring for the current burst.
> + *
> + * @param dev_id
> + * The identifier of the device.
> + * @param vchan
> + * The identifier of virtual DMA channel.
> + *
> + * @return
> + * - Remaining space in the descriptor ring for the current burst on
> success.
> + * - -ENOTSUP: if not supported by the device.
> + */
> +__rte_experimental
> +int
> +rte_dmadev_burst_capacity(uint16_t dev_id, uint16_t vchan);
> +
> /**
> * @warning
> * @b EXPERIMENTAL: this API may change without prior notice.
> diff --git a/lib/dmadev/rte_dmadev_core.h b/lib/dmadev/rte_dmadev_core.h
> index 3c9d698044..2756936798 100644
> --- a/lib/dmadev/rte_dmadev_core.h
> +++ b/lib/dmadev/rte_dmadev_core.h
> @@ -52,6 +52,10 @@ typedef int (*rte_dmadev_stats_get_t)(const struct
> rte_dmadev *dev,
> typedef int (*rte_dmadev_stats_reset_t)(struct rte_dmadev *dev, uint16_t
> vchan);
> /**< @internal Used to reset basic statistics. */
>
> +typedef uint16_t (*rte_dmadev_burst_capacity_t)(const struct rte_dmadev
> *dev,
> + uint16_t vchan);
> +/** < @internal Used to check the remaining space in descriptor ring. */
> +
> typedef int (*rte_dmadev_dump_t)(const struct rte_dmadev *dev, FILE *f);
> /**< @internal Used to dump internal information. */
>
> @@ -114,6 +118,7 @@ struct rte_dmadev_ops {
> rte_dmadev_stats_get_t stats_get;
> rte_dmadev_stats_reset_t stats_reset;
>
> + rte_dmadev_burst_capacity_t burst_capacity;
> rte_dmadev_vchan_status_t vchan_status;
>
> rte_dmadev_dump_t dev_dump;
> diff --git a/lib/dmadev/version.map b/lib/dmadev/version.map
> index 10eeb0f7a3..56cb279e8f 100644
> --- a/lib/dmadev/version.map
> +++ b/lib/dmadev/version.map
> @@ -1,6 +1,7 @@
> EXPERIMENTAL {
> global:
>
> + rte_dmadev_burst_capacity;
> rte_dmadev_close;
> rte_dmadev_completed;
> rte_dmadev_completed_status;
> --
> 2.30.2
>
>
On Wed, Sep 08, 2021 at 11:47:59PM +0530, Jerin Jacob wrote:
> On Tue, 7 Sep 2021, 10:25 pm Bruce Richardson,
> <[1]bruce.richardson@intel.com> wrote:
>
> From: Kevin Laatz <[2]kevin.laatz@intel.com>
> Add a burst capacity check API to the dmadev library. This API is
> useful to
> applications which need to how many descriptors can be enqueued in
> the
> current batch. For example, it could be used to determine whether
> all
> segments of a multi-segment packet can be enqueued in the same batch
> or not
> (to avoid half-offload of the packet).
>
> #Could you share more details on the use case with vhost?
> # Are they planning to use this in fast path if so it need to move as
> fast path function pointer?
I believe the intent is to use it on fastpath, but I would assume only once
per burst, so the penalty for non-fastpath may be acceptable. As you point
out - for an app that really doesn't want to have to pay that penalty,
tracking ring use itself is possible.
The desire for fast-path use is also why I suggested having the space as an
optional return parameter from the submit API call. It could logically also
be a return value from the "completed" call, which might actually make more
sense.
> # Assume the use case needs N rte_dma_copy to complete a logical copy
> at vhost level. Is the any issue in half-offload, meaning when N th one
> successfully completed then only the logical copy is completed. Right?
Yes, as I understand it, the issue is for multi-segment packets, where we
only want to enqueue the first segment if we know we will success with the
final one too.
> # There is already nb_desc with which a dma_queue is configured. So if
> the application does its accounting properly, it knows how many desc it
> has used up and how many completions it has processed.
Agreed. It's just more work for the app, and for simplicity and
completeness I think we should add this API. Because there are other
options I think it should be available, but not as a fast-path fn (though
again, the difference is likely very small for something not called for
every enqueue).
> Would like to understand more details on this API usage.
>
Adding Sunil and Jiayu on CC who are looking at this area from the OVS and
vhost sides.
/Bruce
On Thu, Sep 9, 2021 at 1:46 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Sep 08, 2021 at 11:47:59PM +0530, Jerin Jacob wrote:
> > On Tue, 7 Sep 2021, 10:25 pm Bruce Richardson,
> > <[1]bruce.richardson@intel.com> wrote:
> >
> > From: Kevin Laatz <[2]kevin.laatz@intel.com>
> > Add a burst capacity check API to the dmadev library. This API is
> > useful to
> > applications which need to how many descriptors can be enqueued in
> > the
> > current batch. For example, it could be used to determine whether
> > all
> > segments of a multi-segment packet can be enqueued in the same batch
> > or not
> > (to avoid half-offload of the packet).
> >
> > #Could you share more details on the use case with vhost?
> > # Are they planning to use this in fast path if so it need to move as
> > fast path function pointer?
>
> I believe the intent is to use it on fastpath, but I would assume only once
> per burst, so the penalty for non-fastpath may be acceptable. As you point
> out - for an app that really doesn't want to have to pay that penalty,
> tracking ring use itself is possible.
>
> The desire for fast-path use is also why I suggested having the space as an
> optional return parameter from the submit API call. It could logically also
> be a return value from the "completed" call, which might actually make more
> sense.
>
> > # Assume the use case needs N rte_dma_copy to complete a logical copy
> > at vhost level. Is the any issue in half-offload, meaning when N th one
> > successfully completed then only the logical copy is completed. Right?
>
> Yes, as I understand it, the issue is for multi-segment packets, where we
> only want to enqueue the first segment if we know we will success with the
> final one too.
Sorry for the delay in reply.
If so, why do we need this API. We can mark a logical transaction completed IFF
final segment is succeeded. Since this fastpath API, I would like to
really understand
the real use case for it, so if required then we need to implement in
an optimized way.
Otherwise driver does not need to implement this to have generic solution for
all the drivers.
>
> > # There is already nb_desc with which a dma_queue is configured. So if
> > the application does its accounting properly, it knows how many desc it
> > has used up and how many completions it has processed.
>
> Agreed. It's just more work for the app, and for simplicity and
> completeness I think we should add this API. Because there are other
> options I think it should be available, but not as a fast-path fn (though
> again, the difference is likely very small for something not called for
> every enqueue).
>
> > Would like to understand more details on this API usage.
> >
> Adding Sunil and Jiayu on CC who are looking at this area from the OVS and
> vhost sides.
See above.
Sunil. Jiayu, Could you share the details on the usage and why it is needed.
>
> /Bruce
Hi Jerin,
<snipped>
> > > Add a burst capacity check API to the dmadev library. This API is
> > > useful to
> > > applications which need to how many descriptors can be enqueued in
> > > the
> > > current batch. For example, it could be used to determine whether
> > > all
> > > segments of a multi-segment packet can be enqueued in the same
> batch
> > > or not
> > > (to avoid half-offload of the packet).
> > >
> > > #Could you share more details on the use case with vhost?
> > > # Are they planning to use this in fast path if so it need to move as
> > > fast path function pointer?
> >
> > I believe the intent is to use it on fastpath, but I would assume only
> > once per burst, so the penalty for non-fastpath may be acceptable. As
> > you point out - for an app that really doesn't want to have to pay
> > that penalty, tracking ring use itself is possible.
> >
> > The desire for fast-path use is also why I suggested having the space
> > as an optional return parameter from the submit API call. It could
> > logically also be a return value from the "completed" call, which
> > might actually make more sense.
> >
> > > # Assume the use case needs N rte_dma_copy to complete a logical
> copy
> > > at vhost level. Is the any issue in half-offload, meaning when N th one
> > > successfully completed then only the logical copy is completed. Right?
> >
> > Yes, as I understand it, the issue is for multi-segment packets, where
> > we only want to enqueue the first segment if we know we will success
> > with the final one too.
Yes, this is true. We want to avoid scenarios where only parts of packets could be enqueued.
>
> Sorry for the delay in reply.
>
> If so, why do we need this API. We can mark a logical transaction completed
> IFF final segment is succeeded. Since this fastpath API, I would like to really
> understand the real use case for it, so if required then we need to
> implement in an optimized way.
> Otherwise driver does not need to implement this to have generic solution
> for all the drivers.
>
> >
> > > # There is already nb_desc with which a dma_queue is configured. So if
> > > the application does its accounting properly, it knows how many desc it
> > > has used up and how many completions it has processed.
> >
> > Agreed. It's just more work for the app, and for simplicity and
> > completeness I think we should add this API. Because there are other
> > options I think it should be available, but not as a fast-path fn
> > (though again, the difference is likely very small for something not
> > called for every enqueue).
> >
> > > Would like to understand more details on this API usage.
> > >
> > Adding Sunil and Jiayu on CC who are looking at this area from the OVS
> > and vhost sides.
>
> See above.
>
> Sunil. Jiayu, Could you share the details on the usage and why it is needed.
Here is an example of how the burst capacity API will be potentially used in the app(OVS):
http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4093604-2-sunil.pai.g@intel.com/
Although commented out , it should still provide an idea of its usage.
Thanks and regards,
Sunil
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, September 17, 2021 9:55 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: dpdk-dev <dev@dpdk.org>; Walsh, Conor <conor.walsh@intel.com>;
> Laatz, Kevin <kevin.laatz@intel.com>; fengchengwen
> <fengchengwen@huawei.com>; Jerin Jacob <jerinj@marvell.com>;
> Satananda Burla <sburla@marvell.com>; Radha Mohan Chintakuntla
> <radhac@marvell.com>; Hu, Jiayu <jiayu.hu@intel.com>; Pai G, Sunil
> <sunil.pai.g@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/8] dmadev: add burst capacity API
>
> On Thu, Sep 9, 2021 at 1:46 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, Sep 08, 2021 at 11:47:59PM +0530, Jerin Jacob wrote:
> > > On Tue, 7 Sep 2021, 10:25 pm Bruce Richardson,
> > > <[1]bruce.richardson@intel.com> wrote:
> > >
> > > From: Kevin Laatz <[2]kevin.laatz@intel.com>
> > > Add a burst capacity check API to the dmadev library. This API is
> > > useful to
> > > applications which need to how many descriptors can be enqueued in
> > > the
> > > current batch. For example, it could be used to determine whether
> > > all
> > > segments of a multi-segment packet can be enqueued in the same
> batch
> > > or not
> > > (to avoid half-offload of the packet).
> > >
> > > #Could you share more details on the use case with vhost?
> > > # Are they planning to use this in fast path if so it need to move as
> > > fast path function pointer?
> >
> > I believe the intent is to use it on fastpath, but I would assume only
> > once per burst, so the penalty for non-fastpath may be acceptable. As
> > you point out - for an app that really doesn't want to have to pay
> > that penalty, tracking ring use itself is possible.
> >
> > The desire for fast-path use is also why I suggested having the space
> > as an optional return parameter from the submit API call. It could
> > logically also be a return value from the "completed" call, which
> > might actually make more sense.
> >
> > > # Assume the use case needs N rte_dma_copy to complete a logical
> copy
> > > at vhost level. Is the any issue in half-offload, meaning when N th one
> > > successfully completed then only the logical copy is completed. Right?
> >
> > Yes, as I understand it, the issue is for multi-segment packets, where
> > we only want to enqueue the first segment if we know we will success
> > with the final one too.
>
> Sorry for the delay in reply.
>
> If so, why do we need this API. We can mark a logical transaction completed
> IFF final segment is succeeded. Since this fastpath API, I would like to really
> understand the real use case for it, so if required then we need to implement
> in an optimized way.
> Otherwise driver does not need to implement this to have generic solution
> for all the drivers.
The fact is that it's very hard for apps to calculate the available space of a DMA ring.
For DSA, the available space is decided by three factors: the number of available slots
in SW ring, the max batching size of a batch descriptor, and if there are available batch
descriptors. The first one is configured by SW, and apps can calculate it. But the second
depends on DSA HW, and the third one is hided in DSA driver which is not visible to apps.
Considering the complexity of different DMA HW, I think the best way is to hide all details
inside DMA dev and provide this check capacity API for apps.
Thanks,
Jiayu
>
> >
> > > # There is already nb_desc with which a dma_queue is configured. So if
> > > the application does its accounting properly, it knows how many desc it
> > > has used up and how many completions it has processed.
> >
> > Agreed. It's just more work for the app, and for simplicity and
> > completeness I think we should add this API. Because there are other
> > options I think it should be available, but not as a fast-path fn
> > (though again, the difference is likely very small for something not
> > called for every enqueue).
> >
> > > Would like to understand more details on this API usage.
> > >
> > Adding Sunil and Jiayu on CC who are looking at this area from the OVS
> > and vhost sides.
>
> See above.
>
> Sunil. Jiayu, Could you share the details on the usage and why it is needed.
>
>
> >
> > /Bruce
On Sat, Sep 18, 2021 at 6:36 AM Hu, Jiayu <jiayu.hu@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Friday, September 17, 2021 9:55 PM
> > To: Richardson, Bruce <bruce.richardson@intel.com>
> > Cc: dpdk-dev <dev@dpdk.org>; Walsh, Conor <conor.walsh@intel.com>;
> > Laatz, Kevin <kevin.laatz@intel.com>; fengchengwen
> > <fengchengwen@huawei.com>; Jerin Jacob <jerinj@marvell.com>;
> > Satananda Burla <sburla@marvell.com>; Radha Mohan Chintakuntla
> > <radhac@marvell.com>; Hu, Jiayu <jiayu.hu@intel.com>; Pai G, Sunil
> > <sunil.pai.g@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v3 2/8] dmadev: add burst capacity API
> >
> > On Thu, Sep 9, 2021 at 1:46 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Wed, Sep 08, 2021 at 11:47:59PM +0530, Jerin Jacob wrote:
> > > > On Tue, 7 Sep 2021, 10:25 pm Bruce Richardson,
> > > > <[1]bruce.richardson@intel.com> wrote:
> > > >
> > > > From: Kevin Laatz <[2]kevin.laatz@intel.com>
> > > > Add a burst capacity check API to the dmadev library. This API is
> > > > useful to
> > > > applications which need to how many descriptors can be enqueued in
> > > > the
> > > > current batch. For example, it could be used to determine whether
> > > > all
> > > > segments of a multi-segment packet can be enqueued in the same
> > batch
> > > > or not
> > > > (to avoid half-offload of the packet).
> > > >
> > > > #Could you share more details on the use case with vhost?
> > > > # Are they planning to use this in fast path if so it need to move as
> > > > fast path function pointer?
> > >
> > > I believe the intent is to use it on fastpath, but I would assume only
> > > once per burst, so the penalty for non-fastpath may be acceptable. As
> > > you point out - for an app that really doesn't want to have to pay
> > > that penalty, tracking ring use itself is possible.
> > >
> > > The desire for fast-path use is also why I suggested having the space
> > > as an optional return parameter from the submit API call. It could
> > > logically also be a return value from the "completed" call, which
> > > might actually make more sense.
> > >
> > > > # Assume the use case needs N rte_dma_copy to complete a logical
> > copy
> > > > at vhost level. Is the any issue in half-offload, meaning when N th one
> > > > successfully completed then only the logical copy is completed. Right?
> > >
> > > Yes, as I understand it, the issue is for multi-segment packets, where
> > > we only want to enqueue the first segment if we know we will success
> > > with the final one too.
> >
> > Sorry for the delay in reply.
> >
> > If so, why do we need this API. We can mark a logical transaction completed
> > IFF final segment is succeeded. Since this fastpath API, I would like to really
> > understand the real use case for it, so if required then we need to implement
> > in an optimized way.
> > Otherwise driver does not need to implement this to have generic solution
> > for all the drivers.
Hi Jiayu, Sunil,
> The fact is that it's very hard for apps to calculate the available space of a DMA ring.
Yes, I agree.
My question is more why to calculate the space per burst and introduce
yet another fastpath API.
For example, the application needs to copy 8 segments to complete a
logical copy in the application perspective.
In case, when 8th copy is completed then only the application marks
the logical copy completed.
i.e why to check per burst, 8 segments are available or not? Even it
is available, there may be multiple
reasons why any of the segment copies can fail. So the application
needs to track all the jobs completed
or not anyway. Am I missing something in terms of vhost or OVS usage?
> For DSA, the available space is decided by three factors: the number of available slots
> in SW ring, the max batching size of a batch descriptor, and if there are available batch
> descriptors. The first one is configured by SW, and apps can calculate it. But the second
> depends on DSA HW, and the third one is hided in DSA driver which is not visible to apps.
> Considering the complexity of different DMA HW, I think the best way is to hide all details
> inside DMA dev and provide this check capacity API for apps.
>
> Thanks,
> Jiayu
>
> >
> > >
> > > > # There is already nb_desc with which a dma_queue is configured. So if
> > > > the application does its accounting properly, it knows how many desc it
> > > > has used up and how many completions it has processed.
> > >
> > > Agreed. It's just more work for the app, and for simplicity and
> > > completeness I think we should add this API. Because there are other
> > > options I think it should be available, but not as a fast-path fn
> > > (though again, the difference is likely very small for something not
> > > called for every enqueue).
> > >
> > > > Would like to understand more details on this API usage.
> > > >
> > > Adding Sunil and Jiayu on CC who are looking at this area from the OVS
> > > and vhost sides.
> >
> > See above.
> >
> > Sunil. Jiayu, Could you share the details on the usage and why it is needed.
> >
> >
> > >
> > > /Bruce
On Fri, Sep 17, 2021 at 8:07 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>
> Hi Jerin,
HI Sunil,
>
> <snipped>
>
> > > > Add a burst capacity check API to the dmadev library. This API is
> > > > useful to
> > > > applications which need to how many descriptors can be enqueued in
> > > > the
> > > > current batch. For example, it could be used to determine whether
> > > > all
> > > > segments of a multi-segment packet can be enqueued in the same
> > batch
> > > > or not
> > > > (to avoid half-offload of the packet).
> > > >
> > > > #Could you share more details on the use case with vhost?
> > > > # Are they planning to use this in fast path if so it need to move as
> > > > fast path function pointer?
> > >
> > > I believe the intent is to use it on fastpath, but I would assume only
> > > once per burst, so the penalty for non-fastpath may be acceptable. As
> > > you point out - for an app that really doesn't want to have to pay
> > > that penalty, tracking ring use itself is possible.
> > >
> > > The desire for fast-path use is also why I suggested having the space
> > > as an optional return parameter from the submit API call. It could
> > > logically also be a return value from the "completed" call, which
> > > might actually make more sense.
> > >
> > > > # Assume the use case needs N rte_dma_copy to complete a logical
> > copy
> > > > at vhost level. Is the any issue in half-offload, meaning when N th one
> > > > successfully completed then only the logical copy is completed. Right?
> > >
> > > Yes, as I understand it, the issue is for multi-segment packets, where
> > > we only want to enqueue the first segment if we know we will success
> > > with the final one too.
>
> Yes, this is true. We want to avoid scenarios where only parts of packets could be enqueued.
What are those scenarios, could you share some descriptions of them.
What if the final or any segment fails event the space is available.
So you have to take care of that anyway. RIght?
>
> >
> > Sorry for the delay in reply.
> >
> > If so, why do we need this API. We can mark a logical transaction completed
> > IFF final segment is succeeded. Since this fastpath API, I would like to really
> > understand the real use case for it, so if required then we need to
> > implement in an optimized way.
> > Otherwise driver does not need to implement this to have generic solution
> > for all the drivers.
> >
> > >
> > > > # There is already nb_desc with which a dma_queue is configured. So if
> > > > the application does its accounting properly, it knows how many desc it
> > > > has used up and how many completions it has processed.
> > >
> > > Agreed. It's just more work for the app, and for simplicity and
> > > completeness I think we should add this API. Because there are other
> > > options I think it should be available, but not as a fast-path fn
> > > (though again, the difference is likely very small for something not
> > > called for every enqueue).
> > >
> > > > Would like to understand more details on this API usage.
> > > >
> > > Adding Sunil and Jiayu on CC who are looking at this area from the OVS
> > > and vhost sides.
> >
> > See above.
> >
> > Sunil. Jiayu, Could you share the details on the usage and why it is needed.
>
> Here is an example of how the burst capacity API will be potentially used in the app(OVS):
> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4093604-2-sunil.pai.g@intel.com/
> Although commented out , it should still provide an idea of its usage.
>
> Thanks and regards,
> Sunil
>
Hi Jerin,
> > > > > From: Kevin Laatz <[2]kevin.laatz@intel.com>
> > > > > Add a burst capacity check API to the dmadev library. This API is
> > > > > useful to
> > > > > applications which need to how many descriptors can be enqueued
> in
> > > > > the
> > > > > current batch. For example, it could be used to determine whether
> > > > > all
> > > > > segments of a multi-segment packet can be enqueued in the
> > > > > same
> > > batch
> > > > > or not
> > > > > (to avoid half-offload of the packet).
> > > > >
> > > > > #Could you share more details on the use case with vhost?
> > > > > # Are they planning to use this in fast path if so it need to move as
> > > > > fast path function pointer?
> > > >
> > > > I believe the intent is to use it on fastpath, but I would assume
> > > > only once per burst, so the penalty for non-fastpath may be
> > > > acceptable. As you point out - for an app that really doesn't want
> > > > to have to pay that penalty, tracking ring use itself is possible.
> > > >
> > > > The desire for fast-path use is also why I suggested having the
> > > > space as an optional return parameter from the submit API call. It
> > > > could logically also be a return value from the "completed" call,
> > > > which might actually make more sense.
> > > >
> > > > > # Assume the use case needs N rte_dma_copy to complete a
> > > > > logical
> > > copy
> > > > > at vhost level. Is the any issue in half-offload, meaning when N th
> one
> > > > > successfully completed then only the logical copy is completed.
> Right?
> > > >
> > > > Yes, as I understand it, the issue is for multi-segment packets,
> > > > where we only want to enqueue the first segment if we know we will
> > > > success with the final one too.
> > >
> > > Sorry for the delay in reply.
> > >
> > > If so, why do we need this API. We can mark a logical transaction
> > > completed IFF final segment is succeeded. Since this fastpath API, I
> > > would like to really understand the real use case for it, so if
> > > required then we need to implement in an optimized way.
> > > Otherwise driver does not need to implement this to have generic
> > > solution for all the drivers.
>
>
> Hi Jiayu, Sunil,
>
> > The fact is that it's very hard for apps to calculate the available space of a
> DMA ring.
>
> Yes, I agree.
>
> My question is more why to calculate the space per burst and introduce yet
> another fastpath API.
> For example, the application needs to copy 8 segments to complete a logical
> copy in the application perspective.
> In case, when 8th copy is completed then only the application marks the
> logical copy completed.
> i.e why to check per burst, 8 segments are available or not? Even it is
> available, there may be multiple reasons why any of the segment copies can
> fail. So the application needs to track all the jobs completed or not anyway.
> Am I missing something in terms of vhost or OVS usage?
>
For the packets that do not entirely fit in the DMA ring , we have a SW copy fallback in place.
So, we would like to avoid scenario caused because of DMA ring full where few parts of the packet are copied through DMA and other parts by CPU.
Besides, this API would also help improve debuggability/device introspection to check the occupancy rather than the app having to manually track the state of every DMA device in use.
Copying from other thread:
> What are those scenarios, could you share some descriptions of them.
> What if the final or any segment fails event the space is available.
> So you have to take care of that anyway. RIght?
I think this is app dependent no? The application can choose not to take care of such scenarios and treat the packets as dropped.
Ring full scenarios(-ENOSPC from rte_dma_copy) could be avoided with this API but other errors mean a failure which unfortunately cannot be avoided.
>
> > For DSA, the available space is decided by three factors: the number
> > of available slots in SW ring, the max batching size of a batch
> > descriptor, and if there are available batch descriptors. The first
> > one is configured by SW, and apps can calculate it. But the second depends
> on DSA HW, and the third one is hided in DSA driver which is not visible to
> apps.
> > Considering the complexity of different DMA HW, I think the best way
> > is to hide all details inside DMA dev and provide this check capacity API for
> apps.
> >
> > Thanks,
> > Jiayu
> >
> > >
> > > >
> > > > > # There is already nb_desc with which a dma_queue is configured.
> So if
> > > > > the application does its accounting properly, it knows how many
> desc it
> > > > > has used up and how many completions it has processed.
> > > >
> > > > Agreed. It's just more work for the app, and for simplicity and
> > > > completeness I think we should add this API. Because there are
> > > > other options I think it should be available, but not as a
> > > > fast-path fn (though again, the difference is likely very small
> > > > for something not called for every enqueue).
> > > >
> > > > > Would like to understand more details on this API usage.
> > > > >
> > > > Adding Sunil and Jiayu on CC who are looking at this area from the
> > > > OVS and vhost sides.
> > >
> > > See above.
> > >
> > > Sunil. Jiayu, Could you share the details on the usage and why it is
> needed.
> > >
> > >
> > > >
> > > > /Bruce
On Tue, Sep 21, 2021 at 7:46 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>
> Hi Jerin,
Hi Sunil,
>
> > > > > > From: Kevin Laatz <[2]kevin.laatz@intel.com>
> > > > > > Add a burst capacity check API to the dmadev library. This API is
> > > > > > useful to
> > > > > > applications which need to how many descriptors can be enqueued
> > in
> > > > > > the
> > > > > > current batch. For example, it could be used to determine whether
> > > > > > all
> > > > > > segments of a multi-segment packet can be enqueued in the
> > > > > > same
> > > > batch
> > > > > > or not
> > > > > > (to avoid half-offload of the packet).
> > > > > >
> > > > > > #Could you share more details on the use case with vhost?
> > > > > > # Are they planning to use this in fast path if so it need to move as
> > > > > > fast path function pointer?
> > > > >
> > > > > I believe the intent is to use it on fastpath, but I would assume
> > > > > only once per burst, so the penalty for non-fastpath may be
> > > > > acceptable. As you point out - for an app that really doesn't want
> > > > > to have to pay that penalty, tracking ring use itself is possible.
> > > > >
> > > > > The desire for fast-path use is also why I suggested having the
> > > > > space as an optional return parameter from the submit API call. It
> > > > > could logically also be a return value from the "completed" call,
> > > > > which might actually make more sense.
> > > > >
> > > > > > # Assume the use case needs N rte_dma_copy to complete a
> > > > > > logical
> > > > copy
> > > > > > at vhost level. Is the any issue in half-offload, meaning when N th
> > one
> > > > > > successfully completed then only the logical copy is completed.
> > Right?
> > > > >
> > > > > Yes, as I understand it, the issue is for multi-segment packets,
> > > > > where we only want to enqueue the first segment if we know we will
> > > > > success with the final one too.
> > > >
> > > > Sorry for the delay in reply.
> > > >
> > > > If so, why do we need this API. We can mark a logical transaction
> > > > completed IFF final segment is succeeded. Since this fastpath API, I
> > > > would like to really understand the real use case for it, so if
> > > > required then we need to implement in an optimized way.
> > > > Otherwise driver does not need to implement this to have generic
> > > > solution for all the drivers.
> >
> >
> > Hi Jiayu, Sunil,
> >
> > > The fact is that it's very hard for apps to calculate the available space of a
> > DMA ring.
> >
> > Yes, I agree.
> >
> > My question is more why to calculate the space per burst and introduce yet
> > another fastpath API.
> > For example, the application needs to copy 8 segments to complete a logical
> > copy in the application perspective.
> > In case, when 8th copy is completed then only the application marks the
> > logical copy completed.
> > i.e why to check per burst, 8 segments are available or not? Even it is
> > available, there may be multiple reasons why any of the segment copies can
> > fail. So the application needs to track all the jobs completed or not anyway.
> > Am I missing something in terms of vhost or OVS usage?
> >
>
> For the packets that do not entirely fit in the DMA ring , we have a SW copy fallback in place.
> So, we would like to avoid scenario caused because of DMA ring full where few parts of the packet are copied through DMA and other parts by CPU.
> Besides, this API would also help improve debuggability/device introspection to check the occupancy rather than the app having to manually track the state of every DMA device in use.
To understand it better, Could you share more details on feedback
mechanism on your application enqueue
app_enqueue_v1(.., nb_seg)
{
/* Not enough space, Let application handle it by
dropping or resubmitting */
if (rte_dmadev_burst_capacity() < nb_seg)
return -ENOSPC;
do rte_dma_op() in loop without checking error;
return 0; // Success
}
vs
app_enqueue_v2(.., nb_seg)
{
int rc;
rc |= rte_dma_op() in loop without checking error;
return rc; // return the actual status to application if
Not enough space, Let application handle it by dropping or
resubmitting */
}
Is app_enqueue_v1() and app_enqueue_v2() logically the same from
application PoV. Right?
If not, could you explain, the version you are planning to do for app_enqueue()
> Copying from other thread:
>
> > What are those scenarios, could you share some descriptions of them.
> > What if the final or any segment fails event the space is available.
> > So you have to take care of that anyway. RIght?
>
> I think this is app dependent no? The application can choose not to take care of such scenarios and treat the packets as dropped.
> Ring full scenarios(-ENOSPC from rte_dma_copy) could be avoided with this API but other errors mean a failure which unfortunately cannot be avoided.
>
> >
> > > For DSA, the available space is decided by three factors: the number
> > > of available slots in SW ring, the max batching size of a batch
> > > descriptor, and if there are available batch descriptors. The first
> > > one is configured by SW, and apps can calculate it. But the second depends
> > on DSA HW, and the third one is hided in DSA driver which is not visible to
> > apps.
> > > Considering the complexity of different DMA HW, I think the best way
> > > is to hide all details inside DMA dev and provide this check capacity API for
> > apps.
> > >
> > > Thanks,
> > > Jiayu
> > >
> > > >
> > > > >
> > > > > > # There is already nb_desc with which a dma_queue is configured.
> > So if
> > > > > > the application does its accounting properly, it knows how many
> > desc it
> > > > > > has used up and how many completions it has processed.
> > > > >
> > > > > Agreed. It's just more work for the app, and for simplicity and
> > > > > completeness I think we should add this API. Because there are
> > > > > other options I think it should be available, but not as a
> > > > > fast-path fn (though again, the difference is likely very small
> > > > > for something not called for every enqueue).
> > > > >
> > > > > > Would like to understand more details on this API usage.
> > > > > >
> > > > > Adding Sunil and Jiayu on CC who are looking at this area from the
> > > > > OVS and vhost sides.
> > > >
> > > > See above.
> > > >
> > > > Sunil. Jiayu, Could you share the details on the usage and why it is
> > needed.
> > > >
> > > >
> > > > >
> > > > > /Bruce
Hi Jerin,
<snipped>
> > > > The fact is that it's very hard for apps to calculate the
> > > > available space of a
> > > DMA ring.
> > >
> > > Yes, I agree.
> > >
> > > My question is more why to calculate the space per burst and
> > > introduce yet another fastpath API.
> > > For example, the application needs to copy 8 segments to complete a
> > > logical copy in the application perspective.
> > > In case, when 8th copy is completed then only the application marks
> > > the logical copy completed.
> > > i.e why to check per burst, 8 segments are available or not? Even it
> > > is available, there may be multiple reasons why any of the segment
> > > copies can fail. So the application needs to track all the jobs completed or
> not anyway.
> > > Am I missing something in terms of vhost or OVS usage?
> > >
> >
> > For the packets that do not entirely fit in the DMA ring , we have a SW copy
> fallback in place.
> > So, we would like to avoid scenario caused because of DMA ring full where
> few parts of the packet are copied through DMA and other parts by CPU.
> > Besides, this API would also help improve debuggability/device
> introspection to check the occupancy rather than the app having to manually
> track the state of every DMA device in use.
>
> To understand it better, Could you share more details on feedback
> mechanism on your application enqueue
>
> app_enqueue_v1(.., nb_seg)
> {
> /* Not enough space, Let application handle it by dropping or
> resubmitting */
> if (rte_dmadev_burst_capacity() < nb_seg)
> return -ENOSPC;
>
> do rte_dma_op() in loop without checking error;
> return 0; // Success
> }
>
> vs
> app_enqueue_v2(.., nb_seg)
> {
> int rc;
>
> rc |= rte_dma_op() in loop without checking error;
> return rc; // return the actual status to application if Not enough space,
> Let application handle it by dropping or resubmitting */ }
>
> Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> application PoV. Right?
>
> If not, could you explain, the version you are planning to do for
> app_enqueue()
The exact version can be found here : http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4093604-2-sunil.pai.g@intel.com/
Unfortunately, both versions are not same in our case because of the SW fallback we have for ring full scenario's.
For a packet with 8 nb_segs, if the ring has only space for 4 , we would avoid this packet with app_enqueue_v1
while going ahead with an enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies for a packet which we would want to avoid.
<snipped>
Thanks and regards,
Sunil
On Tue, Sep 21, 2021 at 9:05 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>
> Hi Jerin,
>
> <snipped>
>
> > > > > The fact is that it's very hard for apps to calculate the
> > > > > available space of a
> > > > DMA ring.
> > > >
> > > > Yes, I agree.
> > > >
> > > > My question is more why to calculate the space per burst and
> > > > introduce yet another fastpath API.
> > > > For example, the application needs to copy 8 segments to complete a
> > > > logical copy in the application perspective.
> > > > In case, when 8th copy is completed then only the application marks
> > > > the logical copy completed.
> > > > i.e why to check per burst, 8 segments are available or not? Even it
> > > > is available, there may be multiple reasons why any of the segment
> > > > copies can fail. So the application needs to track all the jobs completed or
> > not anyway.
> > > > Am I missing something in terms of vhost or OVS usage?
> > > >
> > >
> > > For the packets that do not entirely fit in the DMA ring , we have a SW copy
> > fallback in place.
> > > So, we would like to avoid scenario caused because of DMA ring full where
> > few parts of the packet are copied through DMA and other parts by CPU.
> > > Besides, this API would also help improve debuggability/device
> > introspection to check the occupancy rather than the app having to manually
> > track the state of every DMA device in use.
> >
> > To understand it better, Could you share more details on feedback
> > mechanism on your application enqueue
> >
> > app_enqueue_v1(.., nb_seg)
> > {
> > /* Not enough space, Let application handle it by dropping or
> > resubmitting */
> > if (rte_dmadev_burst_capacity() < nb_seg)
> > return -ENOSPC;
> >
> > do rte_dma_op() in loop without checking error;
> > return 0; // Success
> > }
> >
> > vs
> > app_enqueue_v2(.., nb_seg)
> > {
> > int rc;
> >
> > rc |= rte_dma_op() in loop without checking error;
> > return rc; // return the actual status to application if Not enough space,
> > Let application handle it by dropping or resubmitting */ }
> >
> > Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> > application PoV. Right?
> >
> > If not, could you explain, the version you are planning to do for
> > app_enqueue()
>
> The exact version can be found here : http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4093604-2-sunil.pai.g@intel.com/
> Unfortunately, both versions are not same in our case because of the SW fallback we have for ring full scenario's.
> For a packet with 8 nb_segs, if the ring has only space for 4 , we would avoid this packet with app_enqueue_v1
> while going ahead with an enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies for a packet which we would want to avoid.
Thanks for RFC link. Usage is clear now, Since you are checking the
space in the completion handler, I am not sure, what is hard to get
the remaining space,
Will following logic work to find the empty space. If not, please
discuss the issue with this approach. I am trying to avoid yet another
fastpath API
and complication in driver as there is element checking space in the
submit queue and completion queue at least in our hardware.
max_count = nb_desc; (power of 2)
mask = max_count - 1;
for (i = 0; I < n; i++) {
submit_idx = rte_dma_copy();
}
rc = rte_dma_completed(…, &completed_idx..);
space_in_queue = mask - ((submit_idx – completed_idx) & mask);
>
> <snipped>
>
> Thanks and regards,
> Sunil
>
Hi Jerin,
<snipped>
> > > To understand it better, Could you share more details on feedback
> > > mechanism on your application enqueue
> > >
> > > app_enqueue_v1(.., nb_seg)
> > > {
> > > /* Not enough space, Let application handle it by
> > > dropping or resubmitting */
> > > if (rte_dmadev_burst_capacity() < nb_seg)
> > > return -ENOSPC;
> > >
> > > do rte_dma_op() in loop without checking error;
> > > return 0; // Success
> > > }
> > >
> > > vs
> > > app_enqueue_v2(.., nb_seg)
> > > {
> > > int rc;
> > >
> > > rc |= rte_dma_op() in loop without checking error;
> > > return rc; // return the actual status to application
> > > if Not enough space, Let application handle it by dropping or
> > > resubmitting */ }
> > >
> > > Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> > > application PoV. Right?
> > >
> > > If not, could you explain, the version you are planning to do for
> > > app_enqueue()
> >
> > The exact version can be found here :
> >
> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
> > 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
> > same in our case because of the SW fallback we have for ring full scenario's.
> > For a packet with 8 nb_segs, if the ring has only space for 4 , we
> > would avoid this packet with app_enqueue_v1 while going ahead with an
> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
> for a packet which we would want to avoid.
>
> Thanks for RFC link. Usage is clear now, Since you are checking the space in
> the completion handler, I am not sure, what is hard to get the remaining
> space, Will following logic work to find the empty space. If not, please discuss
> the issue with this approach. I am trying to avoid yet another fastpath API
> and complication in driver as there is element checking space in the submit
> queue and completion queue at least in our hardware.
>
> max_count = nb_desc; (power of 2)
> mask = max_count - 1;
>
> for (i = 0; I < n; i++) {
> submit_idx = rte_dma_copy();
> }
> rc = rte_dma_completed(…, &completed_idx..);
> space_in_queue = mask - ((submit_idx – completed_idx) & mask);
>
Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
i.e its pmd dependent.
As Jiayu mentioned before:
> The fact is that it's very hard for apps to calculate the available space of a DMA ring.
> For DSA, the available space is decided by three factors: the number of available slots
> in SW ring, the max batching size of a batch descriptor, and if there are available batch
> descriptors. The first one is configured by SW, and apps can calculate it. But the second
> depends on DSA HW, and the third one is hided in DSA driver which is not visible to apps.
> Considering the complexity of different DMA HW, I think the best way is to hide all details
> inside DMA dev and provide this check capacity API for apps.
<snipped>
Thanks and regards,
Sunil
On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>
> Hi Jerin,
>
> <snipped>
>
> > > > To understand it better, Could you share more details on feedback
> > > > mechanism on your application enqueue
> > > >
> > > > app_enqueue_v1(.., nb_seg)
> > > > {
> > > > /* Not enough space, Let application handle it by
> > > > dropping or resubmitting */
> > > > if (rte_dmadev_burst_capacity() < nb_seg)
> > > > return -ENOSPC;
> > > >
> > > > do rte_dma_op() in loop without checking error;
> > > > return 0; // Success
> > > > }
> > > >
> > > > vs
> > > > app_enqueue_v2(.., nb_seg)
> > > > {
> > > > int rc;
> > > >
> > > > rc |= rte_dma_op() in loop without checking error;
> > > > return rc; // return the actual status to application
> > > > if Not enough space, Let application handle it by dropping or
> > > > resubmitting */ }
> > > >
> > > > Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> > > > application PoV. Right?
> > > >
> > > > If not, could you explain, the version you are planning to do for
> > > > app_enqueue()
> > >
> > > The exact version can be found here :
> > >
> > http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
> > > 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
> > > same in our case because of the SW fallback we have for ring full scenario's.
> > > For a packet with 8 nb_segs, if the ring has only space for 4 , we
> > > would avoid this packet with app_enqueue_v1 while going ahead with an
> > enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
> > for a packet which we would want to avoid.
> >
> > Thanks for RFC link. Usage is clear now, Since you are checking the space in
> > the completion handler, I am not sure, what is hard to get the remaining
> > space, Will following logic work to find the empty space. If not, please discuss
> > the issue with this approach. I am trying to avoid yet another fastpath API
> > and complication in driver as there is element checking space in the submit
> > queue and completion queue at least in our hardware.
> >
> > max_count = nb_desc; (power of 2)
> > mask = max_count - 1;
> >
> > for (i = 0; I < n; i++) {
> > submit_idx = rte_dma_copy();
> > }
> > rc = rte_dma_completed(…, &completed_idx..);
> > space_in_queue = mask - ((submit_idx – completed_idx) & mask);
> >
>
> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
> i.e its pmd dependent.
I did not pay much attention to Jiayu's reply as I did not know what is DSA.
Now I searched the DSA in ml, I could see the PMD patches.
If it is PMD limitation then I am fine with the proposed API.
@Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can
we move to fastpath handler and
remove additional check in fastpath from common code like other APIs.
>
> As Jiayu mentioned before:
> > The fact is that it's very hard for apps to calculate the available space of a DMA ring.
> > For DSA, the available space is decided by three factors: the number of available slots
> > in SW ring, the max batching size of a batch descriptor, and if there are available batch
> > descriptors. The first one is configured by SW, and apps can calculate it. But the second
> > depends on DSA HW, and the third one is hided in DSA driver which is not visible to apps.
> > Considering the complexity of different DMA HW, I think the best way is to hide all details
> > inside DMA dev and provide this check capacity API for apps.
>
> <snipped>
>
> Thanks and regards,
> Sunil
>
On 2021/9/22 2:11, Jerin Jacob wrote:
> On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>>
>> Hi Jerin,
>>
>> <snipped>
>>
>>>>> To understand it better, Could you share more details on feedback
>>>>> mechanism on your application enqueue
>>>>>
>>>>> app_enqueue_v1(.., nb_seg)
>>>>> {
>>>>> /* Not enough space, Let application handle it by
>>>>> dropping or resubmitting */
>>>>> if (rte_dmadev_burst_capacity() < nb_seg)
>>>>> return -ENOSPC;
>>>>>
>>>>> do rte_dma_op() in loop without checking error;
>>>>> return 0; // Success
>>>>> }
>>>>>
>>>>> vs
>>>>> app_enqueue_v2(.., nb_seg)
>>>>> {
>>>>> int rc;
>>>>>
>>>>> rc |= rte_dma_op() in loop without checking error;
>>>>> return rc; // return the actual status to application
>>>>> if Not enough space, Let application handle it by dropping or
>>>>> resubmitting */ }
>>>>>
>>>>> Is app_enqueue_v1() and app_enqueue_v2() logically the same from
>>>>> application PoV. Right?
>>>>>
>>>>> If not, could you explain, the version you are planning to do for
>>>>> app_enqueue()
>>>>
>>>> The exact version can be found here :
>>>>
>>> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
>>>> 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
>>>> same in our case because of the SW fallback we have for ring full scenario's.
>>>> For a packet with 8 nb_segs, if the ring has only space for 4 , we
>>>> would avoid this packet with app_enqueue_v1 while going ahead with an
>>> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
>>> for a packet which we would want to avoid.
>>>
>>> Thanks for RFC link. Usage is clear now, Since you are checking the space in
>>> the completion handler, I am not sure, what is hard to get the remaining
>>> space, Will following logic work to find the empty space. If not, please discuss
>>> the issue with this approach. I am trying to avoid yet another fastpath API
>>> and complication in driver as there is element checking space in the submit
>>> queue and completion queue at least in our hardware.
>>>
>>> max_count = nb_desc; (power of 2)
>>> mask = max_count - 1;
>>>
>>> for (i = 0; I < n; i++) {
>>> submit_idx = rte_dma_copy();
>>> }
>>> rc = rte_dma_completed(…, &completed_idx..);
>>> space_in_queue = mask - ((submit_idx – completed_idx) & mask);
>>>
>>
>> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
>> i.e its pmd dependent.
>
> I did not pay much attention to Jiayu's reply as I did not know what is DSA.
> Now I searched the DSA in ml, I could see the PMD patches.
>
> If it is PMD limitation then I am fine with the proposed API.
>
> @Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can
> we move to fastpath handler and
> remove additional check in fastpath from common code like other APIs.
+1 for move to fastpath.
>
>>
>> As Jiayu mentioned before:
>>> The fact is that it's very hard for apps to calculate the available space of a DMA ring.
>>> For DSA, the available space is decided by three factors: the number of available slots
>>> in SW ring, the max batching size of a batch descriptor, and if there are available batch
>>> descriptors. The first one is configured by SW, and apps can calculate it. But the second
>>> depends on DSA HW, and the third one is hided in DSA driver which is not visible to apps.
>>> Considering the complexity of different DMA HW, I think the best way is to hide all details
>>> inside DMA dev and provide this check capacity API for apps.
>>
>> <snipped>
>>
>> Thanks and regards,
>> Sunil
>>
> .
>
On Wed, Sep 22, 2021 at 09:51:42AM +0800, fengchengwen wrote:
> On 2021/9/22 2:11, Jerin Jacob wrote:
> > On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
> >>
> >> Hi Jerin,
> >>
> >> <snipped>
> >>
> >>>>> To understand it better, Could you share more details on feedback
> >>>>> mechanism on your application enqueue
> >>>>>
> >>>>> app_enqueue_v1(.., nb_seg)
> >>>>> {
> >>>>> /* Not enough space, Let application handle it by
> >>>>> dropping or resubmitting */
> >>>>> if (rte_dmadev_burst_capacity() < nb_seg)
> >>>>> return -ENOSPC;
> >>>>>
> >>>>> do rte_dma_op() in loop without checking error;
> >>>>> return 0; // Success
> >>>>> }
> >>>>>
> >>>>> vs
> >>>>> app_enqueue_v2(.., nb_seg)
> >>>>> {
> >>>>> int rc;
> >>>>>
> >>>>> rc |= rte_dma_op() in loop without checking error;
> >>>>> return rc; // return the actual status to application
> >>>>> if Not enough space, Let application handle it by dropping or
> >>>>> resubmitting */ }
> >>>>>
> >>>>> Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> >>>>> application PoV. Right?
> >>>>>
> >>>>> If not, could you explain, the version you are planning to do for
> >>>>> app_enqueue()
> >>>>
> >>>> The exact version can be found here :
> >>>>
> >>> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
> >>>> 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
> >>>> same in our case because of the SW fallback we have for ring full scenario's.
> >>>> For a packet with 8 nb_segs, if the ring has only space for 4 , we
> >>>> would avoid this packet with app_enqueue_v1 while going ahead with an
> >>> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
> >>> for a packet which we would want to avoid.
> >>>
> >>> Thanks for RFC link. Usage is clear now, Since you are checking the space in
> >>> the completion handler, I am not sure, what is hard to get the remaining
> >>> space, Will following logic work to find the empty space. If not, please discuss
> >>> the issue with this approach. I am trying to avoid yet another fastpath API
> >>> and complication in driver as there is element checking space in the submit
> >>> queue and completion queue at least in our hardware.
> >>>
> >>> max_count = nb_desc; (power of 2)
> >>> mask = max_count - 1;
> >>>
> >>> for (i = 0; I < n; i++) {
> >>> submit_idx = rte_dma_copy();
> >>> }
> >>> rc = rte_dma_completed(…, &completed_idx..);
> >>> space_in_queue = mask - ((submit_idx – completed_idx) & mask);
> >>>
> >>
> >> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
> >> i.e its pmd dependent.
> >
> > I did not pay much attention to Jiayu's reply as I did not know what is DSA.
> > Now I searched the DSA in ml, I could see the PMD patches.
> >
> > If it is PMD limitation then I am fine with the proposed API.
> >
> > @Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can
> > we move to fastpath handler and
> > remove additional check in fastpath from common code like other APIs.
>
> +1 for move to fastpath.
>
Will move in next revision.
On Wed, Sep 22, 2021 at 08:56:50AM +0100, Bruce Richardson wrote:
> On Wed, Sep 22, 2021 at 09:51:42AM +0800, fengchengwen wrote:
> > On 2021/9/22 2:11, Jerin Jacob wrote:
> > > On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
> > >>
> > >> Hi Jerin,
> > >>
> > >> <snipped>
> > >>
> > >>>>> To understand it better, Could you share more details on feedback
> > >>>>> mechanism on your application enqueue
> > >>>>>
> > >>>>> app_enqueue_v1(.., nb_seg)
> > >>>>> {
> > >>>>> /* Not enough space, Let application handle it by
> > >>>>> dropping or resubmitting */
> > >>>>> if (rte_dmadev_burst_capacity() < nb_seg)
> > >>>>> return -ENOSPC;
> > >>>>>
> > >>>>> do rte_dma_op() in loop without checking error;
> > >>>>> return 0; // Success
> > >>>>> }
> > >>>>>
> > >>>>> vs
> > >>>>> app_enqueue_v2(.., nb_seg)
> > >>>>> {
> > >>>>> int rc;
> > >>>>>
> > >>>>> rc |= rte_dma_op() in loop without checking error;
> > >>>>> return rc; // return the actual status to application
> > >>>>> if Not enough space, Let application handle it by dropping or
> > >>>>> resubmitting */ }
> > >>>>>
> > >>>>> Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> > >>>>> application PoV. Right?
> > >>>>>
> > >>>>> If not, could you explain, the version you are planning to do for
> > >>>>> app_enqueue()
> > >>>>
> > >>>> The exact version can be found here :
> > >>>>
> > >>> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
> > >>>> 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
> > >>>> same in our case because of the SW fallback we have for ring full scenario's.
> > >>>> For a packet with 8 nb_segs, if the ring has only space for 4 , we
> > >>>> would avoid this packet with app_enqueue_v1 while going ahead with an
> > >>> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
> > >>> for a packet which we would want to avoid.
> > >>>
> > >>> Thanks for RFC link. Usage is clear now, Since you are checking the space in
> > >>> the completion handler, I am not sure, what is hard to get the remaining
> > >>> space, Will following logic work to find the empty space. If not, please discuss
> > >>> the issue with this approach. I am trying to avoid yet another fastpath API
> > >>> and complication in driver as there is element checking space in the submit
> > >>> queue and completion queue at least in our hardware.
> > >>>
> > >>> max_count = nb_desc; (power of 2)
> > >>> mask = max_count - 1;
> > >>>
> > >>> for (i = 0; I < n; i++) {
> > >>> submit_idx = rte_dma_copy();
> > >>> }
> > >>> rc = rte_dma_completed(…, &completed_idx..);
> > >>> space_in_queue = mask - ((submit_idx – completed_idx) & mask);
> > >>>
> > >>
> > >> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
> > >> i.e its pmd dependent.
> > >
> > > I did not pay much attention to Jiayu's reply as I did not know what is DSA.
> > > Now I searched the DSA in ml, I could see the PMD patches.
> > >
> > > If it is PMD limitation then I am fine with the proposed API.
> > >
> > > @Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can
> > > we move to fastpath handler and
> > > remove additional check in fastpath from common code like other APIs.
> >
> > +1 for move to fastpath.
> >
>
> Will move in next revision.
Follow-up question on this. If it's a fastpath function then we would not
normally check for support from drivers. Therefore do we want to:
1. make it a mandatory function
2. add a feature capability flag
Given that it's likely fairly easy for all drivers to implement, and it
makes it easier for apps to avoid having to check a feature flag for, I'd
tend towards option #1, but just would like consensus before I push any
more patches.
/Bruce
On Wed, Sep 22, 2021 at 10:06 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Sep 22, 2021 at 08:56:50AM +0100, Bruce Richardson wrote:
> > On Wed, Sep 22, 2021 at 09:51:42AM +0800, fengchengwen wrote:
> > > On 2021/9/22 2:11, Jerin Jacob wrote:
> > > > On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
> > > >>
> > > >> Hi Jerin,
> > > >>
> > > >> <snipped>
> > > >>
> > > >>>>> To understand it better, Could you share more details on feedback
> > > >>>>> mechanism on your application enqueue
> > > >>>>>
> > > >>>>> app_enqueue_v1(.., nb_seg)
> > > >>>>> {
> > > >>>>> /* Not enough space, Let application handle it by
> > > >>>>> dropping or resubmitting */
> > > >>>>> if (rte_dmadev_burst_capacity() < nb_seg)
> > > >>>>> return -ENOSPC;
> > > >>>>>
> > > >>>>> do rte_dma_op() in loop without checking error;
> > > >>>>> return 0; // Success
> > > >>>>> }
> > > >>>>>
> > > >>>>> vs
> > > >>>>> app_enqueue_v2(.., nb_seg)
> > > >>>>> {
> > > >>>>> int rc;
> > > >>>>>
> > > >>>>> rc |= rte_dma_op() in loop without checking error;
> > > >>>>> return rc; // return the actual status to application
> > > >>>>> if Not enough space, Let application handle it by dropping or
> > > >>>>> resubmitting */ }
> > > >>>>>
> > > >>>>> Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> > > >>>>> application PoV. Right?
> > > >>>>>
> > > >>>>> If not, could you explain, the version you are planning to do for
> > > >>>>> app_enqueue()
> > > >>>>
> > > >>>> The exact version can be found here :
> > > >>>>
> > > >>> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
> > > >>>> 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
> > > >>>> same in our case because of the SW fallback we have for ring full scenario's.
> > > >>>> For a packet with 8 nb_segs, if the ring has only space for 4 , we
> > > >>>> would avoid this packet with app_enqueue_v1 while going ahead with an
> > > >>> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
> > > >>> for a packet which we would want to avoid.
> > > >>>
> > > >>> Thanks for RFC link. Usage is clear now, Since you are checking the space in
> > > >>> the completion handler, I am not sure, what is hard to get the remaining
> > > >>> space, Will following logic work to find the empty space. If not, please discuss
> > > >>> the issue with this approach. I am trying to avoid yet another fastpath API
> > > >>> and complication in driver as there is element checking space in the submit
> > > >>> queue and completion queue at least in our hardware.
> > > >>>
> > > >>> max_count = nb_desc; (power of 2)
> > > >>> mask = max_count - 1;
> > > >>>
> > > >>> for (i = 0; I < n; i++) {
> > > >>> submit_idx = rte_dma_copy();
> > > >>> }
> > > >>> rc = rte_dma_completed(…, &completed_idx..);
> > > >>> space_in_queue = mask - ((submit_idx – completed_idx) & mask);
> > > >>>
> > > >>
> > > >> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
> > > >> i.e its pmd dependent.
> > > >
> > > > I did not pay much attention to Jiayu's reply as I did not know what is DSA.
> > > > Now I searched the DSA in ml, I could see the PMD patches.
> > > >
> > > > If it is PMD limitation then I am fine with the proposed API.
> > > >
> > > > @Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can
> > > > we move to fastpath handler and
> > > > remove additional check in fastpath from common code like other APIs.
> > >
> > > +1 for move to fastpath.
> > >
> >
> > Will move in next revision.
>
> Follow-up question on this. If it's a fastpath function then we would not
> normally check for support from drivers. Therefore do we want to:
> 1. make it a mandatory function
> 2. add a feature capability flag
>
> Given that it's likely fairly easy for all drivers to implement, and it
> makes it easier for apps to avoid having to check a feature flag for, I'd
> tend towards option #1, but just would like consensus before I push any
> more patches.
I think, if vhost using in that way then it needs to make it a
mandatory function.
>
> /Bruce
On 2021/9/23 1:29, Jerin Jacob wrote:
> On Wed, Sep 22, 2021 at 10:06 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
>>
>> On Wed, Sep 22, 2021 at 08:56:50AM +0100, Bruce Richardson wrote:
>>> On Wed, Sep 22, 2021 at 09:51:42AM +0800, fengchengwen wrote:
>>>> On 2021/9/22 2:11, Jerin Jacob wrote:
>>>>> On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>>>>>>
>>>>>> Hi Jerin,
>>>>>>
>>>>>> <snipped>
>>>>>>
>>>>>>>>> To understand it better, Could you share more details on feedback
>>>>>>>>> mechanism on your application enqueue
>>>>>>>>>
>>>>>>>>> app_enqueue_v1(.., nb_seg)
>>>>>>>>> {
>>>>>>>>> /* Not enough space, Let application handle it by
>>>>>>>>> dropping or resubmitting */
>>>>>>>>> if (rte_dmadev_burst_capacity() < nb_seg)
>>>>>>>>> return -ENOSPC;
>>>>>>>>>
>>>>>>>>> do rte_dma_op() in loop without checking error;
>>>>>>>>> return 0; // Success
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> vs
>>>>>>>>> app_enqueue_v2(.., nb_seg)
>>>>>>>>> {
>>>>>>>>> int rc;
>>>>>>>>>
>>>>>>>>> rc |= rte_dma_op() in loop without checking error;
>>>>>>>>> return rc; // return the actual status to application
>>>>>>>>> if Not enough space, Let application handle it by dropping or
>>>>>>>>> resubmitting */ }
>>>>>>>>>
>>>>>>>>> Is app_enqueue_v1() and app_enqueue_v2() logically the same from
>>>>>>>>> application PoV. Right?
>>>>>>>>>
>>>>>>>>> If not, could you explain, the version you are planning to do for
>>>>>>>>> app_enqueue()
>>>>>>>>
>>>>>>>> The exact version can be found here :
>>>>>>>>
>>>>>>> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
>>>>>>>> 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
>>>>>>>> same in our case because of the SW fallback we have for ring full scenario's.
>>>>>>>> For a packet with 8 nb_segs, if the ring has only space for 4 , we
>>>>>>>> would avoid this packet with app_enqueue_v1 while going ahead with an
>>>>>>> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
>>>>>>> for a packet which we would want to avoid.
>>>>>>>
>>>>>>> Thanks for RFC link. Usage is clear now, Since you are checking the space in
>>>>>>> the completion handler, I am not sure, what is hard to get the remaining
>>>>>>> space, Will following logic work to find the empty space. If not, please discuss
>>>>>>> the issue with this approach. I am trying to avoid yet another fastpath API
>>>>>>> and complication in driver as there is element checking space in the submit
>>>>>>> queue and completion queue at least in our hardware.
>>>>>>>
>>>>>>> max_count = nb_desc; (power of 2)
>>>>>>> mask = max_count - 1;
>>>>>>>
>>>>>>> for (i = 0; I < n; i++) {
>>>>>>> submit_idx = rte_dma_copy();
>>>>>>> }
>>>>>>> rc = rte_dma_completed(…, &completed_idx..);
>>>>>>> space_in_queue = mask - ((submit_idx – completed_idx) & mask);
>>>>>>>
>>>>>>
>>>>>> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
>>>>>> i.e its pmd dependent.
>>>>>
>>>>> I did not pay much attention to Jiayu's reply as I did not know what is DSA.
>>>>> Now I searched the DSA in ml, I could see the PMD patches.
>>>>>
>>>>> If it is PMD limitation then I am fine with the proposed API.
>>>>>
>>>>> @Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can
>>>>> we move to fastpath handler and
>>>>> remove additional check in fastpath from common code like other APIs.
>>>>
>>>> +1 for move to fastpath.
>>>>
>>>
>>> Will move in next revision.
>>
>> Follow-up question on this. If it's a fastpath function then we would not
>> normally check for support from drivers. Therefore do we want to:
>> 1. make it a mandatory function
>> 2. add a feature capability flag
>>
>> Given that it's likely fairly easy for all drivers to implement, and it
>> makes it easier for apps to avoid having to check a feature flag for, I'd
>> tend towards option #1, but just would like consensus before I push any
>> more patches.
>
> I think, if vhost using in that way then it needs to make it a
> mandatory function.
+1
>
>>
>> /Bruce
> .
>
@@ -573,6 +573,17 @@ dmadev_dump_capability(FILE *f, uint64_t dev_capa)
fprintf(f, "\n");
}
+int
+rte_dmadev_burst_capacity(uint16_t dev_id, uint16_t vchan)
+{
+ const struct rte_dmadev *dev = &rte_dmadevices[dev_id];
+
+ RTE_DMADEV_VALID_DEV_ID_OR_ERR_RET(dev_id, -EINVAL);
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->burst_capacity, -ENOTSUP);
+ return (*dev->dev_ops->burst_capacity)(dev, vchan);
+}
+
int
rte_dmadev_dump(uint16_t dev_id, FILE *f)
{
@@ -673,6 +673,25 @@ __rte_experimental
int
rte_dmadev_vchan_status(uint16_t dev_id, uint16_t vchan, enum rte_dmadev_vchan_status *status);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Check remaining capacity in descriptor ring for the current burst.
+ *
+ * @param dev_id
+ * The identifier of the device.
+ * @param vchan
+ * The identifier of virtual DMA channel.
+ *
+ * @return
+ * - Remaining space in the descriptor ring for the current burst on success.
+ * - -ENOTSUP: if not supported by the device.
+ */
+__rte_experimental
+int
+rte_dmadev_burst_capacity(uint16_t dev_id, uint16_t vchan);
+
/**
* @warning
* @b EXPERIMENTAL: this API may change without prior notice.
@@ -52,6 +52,10 @@ typedef int (*rte_dmadev_stats_get_t)(const struct rte_dmadev *dev,
typedef int (*rte_dmadev_stats_reset_t)(struct rte_dmadev *dev, uint16_t vchan);
/**< @internal Used to reset basic statistics. */
+typedef uint16_t (*rte_dmadev_burst_capacity_t)(const struct rte_dmadev *dev,
+ uint16_t vchan);
+/** < @internal Used to check the remaining space in descriptor ring. */
+
typedef int (*rte_dmadev_dump_t)(const struct rte_dmadev *dev, FILE *f);
/**< @internal Used to dump internal information. */
@@ -114,6 +118,7 @@ struct rte_dmadev_ops {
rte_dmadev_stats_get_t stats_get;
rte_dmadev_stats_reset_t stats_reset;
+ rte_dmadev_burst_capacity_t burst_capacity;
rte_dmadev_vchan_status_t vchan_status;
rte_dmadev_dump_t dev_dump;
@@ -1,6 +1,7 @@
EXPERIMENTAL {
global:
+ rte_dmadev_burst_capacity;
rte_dmadev_close;
rte_dmadev_completed;
rte_dmadev_completed_status;