DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerinjacobk@gmail.com>
To: "Pai G, Sunil" <sunil.pai.g@intel.com>
Cc: "Hu, Jiayu" <jiayu.hu@intel.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	 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>
Subject: Re: [dpdk-dev] [PATCH v3 2/8] dmadev: add burst capacity API
Date: Tue, 21 Sep 2021 20:26:33 +0530	[thread overview]
Message-ID: <CALBAE1M_hNksUZHYgRSvORr_w=YoxVd4QF3UhT5YxUBzU0QtUA@mail.gmail.com> (raw)
In-Reply-To: <BYAPR11MB3814C84C5DA950B74DCACB82BDA19@BYAPR11MB3814.namprd11.prod.outlook.com>

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

  reply	other threads:[~2021-09-21 14:57 UTC|newest]

Thread overview: 130+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 18:32 [dpdk-dev] [RFC PATCH 0/7] add test suite for DMA drivers Bruce Richardson
2021-08-26 18:32 ` [dpdk-dev] [RFC PATCH 1/7] app/test: take API tests from skeleton dmadev Bruce Richardson
2021-08-26 18:32 ` [dpdk-dev] [RFC PATCH 2/7] dmadev: remove selftest support Bruce Richardson
2021-08-26 18:32 ` [dpdk-dev] [RFC PATCH 3/7] app/test: add basic dmadev instance tests Bruce Richardson
2021-08-26 18:32 ` [dpdk-dev] [RFC PATCH 4/7] app/test: add basic dmadev copy tests Bruce Richardson
2021-08-27  7:14   ` Jerin Jacob
2021-08-27 10:41     ` Bruce Richardson
2021-08-26 18:32 ` [dpdk-dev] [RFC PATCH 5/7] app/test: add more comprehensive " Bruce Richardson
2021-08-26 18:33 ` [dpdk-dev] [RFC PATCH 6/7] app/test: test dmadev instance failure handling Bruce Richardson
2021-08-26 18:33 ` [dpdk-dev] [RFC PATCH 7/7] app/test: add dmadev fill tests Bruce Richardson
2021-09-01 16:32 ` [dpdk-dev] [PATCH v2 0/6] add test suite for DMA drivers Bruce Richardson
2021-09-01 16:32   ` [dpdk-dev] [PATCH v2 1/6] dmadev: add device idle check for testing use Bruce Richardson
2021-09-02 12:54     ` fengchengwen
2021-09-02 14:21       ` Bruce Richardson
2021-09-01 16:32   ` [dpdk-dev] [PATCH v2 2/6] app/test: add basic dmadev instance tests Bruce Richardson
2021-09-01 19:24     ` Mattias Rönnblom
2021-09-02 10:30       ` Bruce Richardson
2021-09-03 16:07     ` Conor Walsh
2021-09-01 16:32   ` [dpdk-dev] [PATCH v2 3/6] app/test: add basic dmadev copy tests Bruce Richardson
2021-09-02  7:44     ` Jerin Jacob
2021-09-02  8:06       ` Bruce Richardson
2021-09-02 10:54         ` Jerin Jacob
2021-09-02 11:43           ` Bruce Richardson
2021-09-02 13:05             ` Jerin Jacob
2021-09-02 14:21               ` Bruce Richardson
2021-09-03 16:05     ` Kevin Laatz
2021-09-03 16:07     ` Conor Walsh
2021-09-01 16:32   ` [dpdk-dev] [PATCH v2 4/6] app/test: add more comprehensive " Bruce Richardson
2021-09-03 16:08     ` Conor Walsh
2021-09-03 16:11     ` Kevin Laatz
2021-09-01 16:32   ` [dpdk-dev] [PATCH v2 5/6] app/test: test dmadev instance failure handling Bruce Richardson
2021-09-01 19:53     ` Mattias Rönnblom
2021-09-03 16:08     ` Conor Walsh
2021-09-03 16:21     ` Kevin Laatz
2021-09-01 16:32   ` [dpdk-dev] [PATCH v2 6/6] app/test: add dmadev fill tests Bruce Richardson
2021-09-03 16:09     ` Conor Walsh
2021-09-03 16:17       ` Conor Walsh
2021-09-03 16:33         ` Bruce Richardson
2021-09-07 16:49 ` [dpdk-dev] [PATCH v3 0/8] add test suite for DMA drivers Bruce Richardson
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 1/8] dmadev: add channel status check for testing use Bruce Richardson
2021-09-08 10:50     ` Walsh, Conor
2021-09-08 13:20     ` Kevin Laatz
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 2/8] dmadev: add burst capacity API Bruce Richardson
2021-09-08 10:53     ` Walsh, Conor
2021-09-08 18:17     ` Jerin Jacob
2021-09-09  8:16       ` Bruce Richardson
2021-09-17 13:54         ` Jerin Jacob
2021-09-17 14:37           ` Pai G, Sunil
2021-09-18 12:18             ` Jerin Jacob
2021-09-18  1:06           ` Hu, Jiayu
2021-09-18 12:12             ` Jerin Jacob
2021-09-21 13:57               ` Pai G, Sunil
2021-09-21 14:56                 ` Jerin Jacob [this message]
2021-09-21 15:34                   ` Pai G, Sunil
2021-09-21 16:58                     ` Jerin Jacob
2021-09-21 17:12                       ` Pai G, Sunil
2021-09-21 18:11                         ` Jerin Jacob
2021-09-22  1:51                           ` fengchengwen
2021-09-22  7:56                             ` Bruce Richardson
2021-09-22 16:35                               ` Bruce Richardson
2021-09-22 17:29                                 ` Jerin Jacob
2021-09-23 13:24                                   ` fengchengwen
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 3/8] app/test: add basic dmadev instance tests Bruce Richardson
2021-09-08 13:21     ` Kevin Laatz
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 4/8] app/test: add basic dmadev copy tests Bruce Richardson
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 5/8] app/test: add more comprehensive " Bruce Richardson
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 6/8] app/test: test dmadev instance failure handling Bruce Richardson
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 7/8] app/test: add dmadev fill tests Bruce Richardson
2021-09-07 16:49   ` [dpdk-dev] [PATCH v3 8/8] app/test: add dmadev burst capacity API test Bruce Richardson
2021-09-08 11:03     ` Walsh, Conor
2021-09-17 13:30 ` [dpdk-dev] [PATCH v4 0/9] add test suite for DMA drivers Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 1/9] dmadev: add channel status check for testing use Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 2/9] dmadev: add burst capacity API Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 3/9] dmadev: add device iterator Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 4/9] app/test: add basic dmadev instance tests Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 5/9] app/test: add basic dmadev copy tests Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 6/9] app/test: add more comprehensive " Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 7/9] app/test: test dmadev instance failure handling Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 8/9] app/test: add dmadev fill tests Bruce Richardson
2021-09-17 13:30   ` [dpdk-dev] [PATCH v4 9/9] app/test: add dmadev burst capacity API test Bruce Richardson
2021-09-17 13:54 ` [dpdk-dev] [PATCH v5 0/9] add test suite for DMA drivers Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 1/9] dmadev: add channel status check for testing use Bruce Richardson
2021-09-22  8:25     ` fengchengwen
2021-09-22  8:31       ` Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 2/9] dmadev: add burst capacity API Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 3/9] dmadev: add device iterator Bruce Richardson
2021-09-22  8:46     ` fengchengwen
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 4/9] app/test: add basic dmadev instance tests Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 5/9] app/test: add basic dmadev copy tests Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 6/9] app/test: add more comprehensive " Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 7/9] app/test: test dmadev instance failure handling Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 8/9] app/test: add dmadev fill tests Bruce Richardson
2021-09-17 13:54   ` [dpdk-dev] [PATCH v5 9/9] app/test: add dmadev burst capacity API test Bruce Richardson
2021-09-24 10:29 ` [dpdk-dev] [PATCH v6 00/13] add test suite for DMA drivers Bruce Richardson
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 01/13] dmadev: add channel status check for testing use Bruce Richardson
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 02/13] dma/skeleton: add channel status function Bruce Richardson
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 03/13] dmadev: add burst capacity API Bruce Richardson
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 04/13] dma/skeleton: add burst capacity function Bruce Richardson
2021-09-24 14:51     ` Conor Walsh
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 05/13] dmadev: add device iterator Bruce Richardson
2021-09-24 14:52     ` Conor Walsh
2021-09-24 15:58     ` Kevin Laatz
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 06/13] app/test: add basic dmadev instance tests Bruce Richardson
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 07/13] app/test: add basic dmadev copy tests Bruce Richardson
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 08/13] app/test: run test suite on skeleton driver Bruce Richardson
2021-09-24 15:58     ` Kevin Laatz
2021-09-24 10:29   ` [dpdk-dev] [PATCH v6 09/13] app/test: add more comprehensive dmadev copy tests Bruce Richardson
2021-09-24 10:31   ` [dpdk-dev] [PATCH v6 10/13] dmadev: add flag for error handling support Bruce Richardson
2021-09-24 14:52     ` Conor Walsh
2021-09-24 15:58     ` Kevin Laatz
2021-09-24 10:31   ` [dpdk-dev] [PATCH v6 11/13] app/test: test dmadev instance failure handling Bruce Richardson
2021-09-24 10:31   ` [dpdk-dev] [PATCH v6 12/13] app/test: add dmadev fill tests Bruce Richardson
2021-09-24 10:31   ` [dpdk-dev] [PATCH v6 13/13] app/test: add dmadev burst capacity API test Bruce Richardson
2021-10-13 15:17   ` [dpdk-dev] [PATCH v7 00/13] add test suite for DMA drivers Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 01/13] dmadev: add channel status check for testing use Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 02/13] dma/skeleton: add channel status function Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 03/13] dmadev: add burst capacity API Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 04/13] dma/skeleton: add burst capacity function Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 05/13] dmadev: add device iterator Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 06/13] app/test: add basic dmadev instance tests Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 07/13] app/test: add basic dmadev copy tests Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 08/13] app/test: run test suite on skeleton driver Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 09/13] app/test: add more comprehensive dmadev copy tests Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 10/13] dmadev: add flag for error handling support Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 11/13] app/test: test dmadev instance failure handling Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 12/13] app/test: add dmadev fill tests Bruce Richardson
2021-10-13 15:17     ` [dpdk-dev] [PATCH v7 13/13] app/test: add dmadev burst capacity API test Bruce Richardson
2021-10-18  9:20     ` [dpdk-dev] [PATCH v7 00/13] add test suite for DMA drivers Thomas Monjalon
2021-10-21 12:06       ` fengchengwen
2021-10-21 14:55         ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALBAE1M_hNksUZHYgRSvORr_w=YoxVd4QF3UhT5YxUBzU0QtUA@mail.gmail.com' \
    --to=jerinjacobk@gmail.com \
    --cc=bruce.richardson@intel.com \
    --cc=conor.walsh@intel.com \
    --cc=dev@dpdk.org \
    --cc=fengchengwen@huawei.com \
    --cc=jerinj@marvell.com \
    --cc=jiayu.hu@intel.com \
    --cc=kevin.laatz@intel.com \
    --cc=radhac@marvell.com \
    --cc=sburla@marvell.com \
    --cc=sunil.pai.g@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).