From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 78C30A0C47;
Thu, 2 Sep 2021 12:54:47 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id EC5DC4003E;
Thu, 2 Sep 2021 12:54:46 +0200 (CEST)
Received: from mail-il1-f176.google.com (mail-il1-f176.google.com
[209.85.166.176])
by mails.dpdk.org (Postfix) with ESMTP id 8D3784003C
for <dev@dpdk.org>; Thu, 2 Sep 2021 12:54:45 +0200 (CEST)
Received: by mail-il1-f176.google.com with SMTP id u7so1305605ilk.7
for <dev@dpdk.org>; Thu, 02 Sep 2021 03:54:45 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;
h=mime-version:references:in-reply-to:from:date:message-id:subject:to
:cc; bh=d/M3Fkpvok+NpsQthBS+l/L0fGduZahc63DEd0jI8Jo=;
b=HHL7Ssnhj8yAoKDBLkbDJ512SAXVw9rs7ESsz0LfpTIWAVk75DmX0t8M5YnBGz+0DA
fYN29ItYKhjMWPXliui3KoCRxMpMySG3eppOK4xU+lJbxDXJ3DxvuOHygiB9M3lnjWZg
YF5AWE68fqms+OEUKxcW1ISD7hhygxVL2QD/mLAJq6/DURsd2g95wLRYcgu5M0+g+n5o
VmFXoMOmshfyuXm/5SOTw9UJrLAgIpQoilsWfvvIEmJw2DhuwGwaOdDmWRgso1hrPrz5
XnwSeIUudmnpEWRbLGauWIZdSCl8bEjyc3S4IRtL/77WWTbzd6IheiGu+BFHpMtgiSmf
/OZg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=d/M3Fkpvok+NpsQthBS+l/L0fGduZahc63DEd0jI8Jo=;
b=ir4RD6pPbkIEf5MflA8WXnOc+dsbZ30wwSxJVD9ek52yzJ3BNfgRGn/ntc90EwBiGl
uemDie65fEkyaOv/FpML7qLfaiQx7MoJT99ptWFedOwxORlyX1IuLA0xFq6f/9D6LdPV
eEGDnoonCB6k9VZCRgPHrUriFzQn8NW+LQrdxms66LBkyJ+zwt6LtzGTFwamSugtmNQB
G+/Ok8k5BC1ELbl3IxnRPqsv1HbdNZ3Cp+hMJblZnrZGamDDC9aXlqOsRDvdpuwS7gS+
35gR0V3EzeWEqousLoybEO5u58GMzMdPFBc/IG2M4+1raXjA/8MtDchB7RdMkHO1ZV7Z
Cuxw==
X-Gm-Message-State: AOAM530AzoakegA/kw0pH6LKg26aGye5DBkFzidg25FO/W2qolsSJktc
xXjzwaAdHQT+T8MmKVtu3kLTH/uM0Ydde7+j85s=
X-Google-Smtp-Source: ABdhPJy6rjzYh5jngqg9ZExLx6j07eD/1eS4Hi6TwX69LPX/0cwM5yJWCuqxk1a3LBhQyAmkYTrRLlfbha6sYD9+G1g=
X-Received: by 2002:a05:6e02:160f:: with SMTP id
t15mr1978330ilu.60.1630580084869;
Thu, 02 Sep 2021 03:54:44 -0700 (PDT)
MIME-Version: 1.0
References: <20210826183301.333442-1-bruce.richardson@intel.com>
<20210901163216.120087-1-bruce.richardson@intel.com>
<20210901163216.120087-4-bruce.richardson@intel.com>
<CALBAE1P-84=KOyiiqZvb8aD=Yg2qVvXtNnwnUnSCBHDUeWsvxA@mail.gmail.com>
<YTCGAua0r+j/Z6Mw@bricha3-MOBL.ger.corp.intel.com>
In-Reply-To: <YTCGAua0r+j/Z6Mw@bricha3-MOBL.ger.corp.intel.com>
From: Jerin Jacob <jerinjacobk@gmail.com>
Date: Thu, 2 Sep 2021 16:24:18 +0530
Message-ID: <CALBAE1NymsapE5DvkJx5h_2PVPWVv8hKEAVQACDs3uC955=0hA@mail.gmail.com>
To: Bruce Richardson <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>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v2 3/6] app/test: add basic dmadev copy tests
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
On Thu, Sep 2, 2021 at 1:36 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Sep 02, 2021 at 01:14:38PM +0530, Jerin Jacob wrote:
> > On Wed, Sep 1, 2021 at 10:02 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > For each dmadev instance, perform some basic copy tests to validate that
> > > functionality.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > > app/test/test_dmadev.c | 174 +++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 174 insertions(+)
> > >
> > > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> > > index 12f7c69629..261f45db71 100644
> > > --- a/app/test/test_dmadev.c
> > > +++ b/app/test/test_dmadev.c
> > > @@ -2,12 +2,15 @@
> > > * Copyright(c) 2021 HiSilicon Limited.
> > > * Copyright(c) 2021 Intel Corporation.
> > > */
> > > +#include <unistd.h>
> > > #include <inttypes.h>
> > >
> > > #include <rte_common.h>
> > > #include <rte_dev.h>
> > > #include <rte_dmadev.h>
> > > #include <rte_bus_vdev.h>
> > > +#include <rte_mbuf.h>
> > > +#include <rte_random.h>
> > >
> > > #include "test.h"
> > >
> > > @@ -16,6 +19,11 @@ extern int test_dmadev_api(uint16_t dev_id);
> > >
> > > #define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
> > >
> > > +#define COPY_LEN 1024
> > > +
> > > +static struct rte_mempool *pool;
> > > +static uint16_t id_count;
> > > +
> > > static inline int
> > > __rte_format_printf(3, 4)
> > > print_err(const char *func, int lineno, const char *format, ...)
> > > @@ -31,6 +39,134 @@ print_err(const char *func, int lineno, const char *format, ...)
> > > return ret;
> > > }
> > >
> > > +static inline void
> > > +await_hw(int dev_id, uint16_t vchan)
> > > +{
> > > + int idle = rte_dmadev_vchan_idle(dev_id, vchan);
> > > + if (idle < 0) {
> > > + /* for drivers that don't support this op, just sleep for 25 microseconds */
> > > + usleep(25);
> > > + return;
> > > + }
> >
> > Can following model eliminate the need for rte_dmadev_vchan_idle() API. Right?
> >
> > static inline bool
> > await_hw(int dev_id, uint16_t vchan, uint16_t nb_req, uint16_t *last_idx)
> > {
> > const uint64_t tmo = rte_get_timer_hz();
> > bool has_error = false;
> >
> > const uint64_t end_cycles = rte_get_timer_cycles() + tmo;
> > while (rte_get_timer_cycles() < end_cycles && nb_req > 0
> > && has_error == false) {
> > rte_pause();
> > nb_req -= rte_dmadev_completed(dev_id,
> > nb_req, last_idx, &has_error);
> > }
> >
> > return has_error ;
> > }
> >
> It would, but unfortunately it also removes the possibility of doing a
> number of the tests in the set, particularly around failure handling. We
> used runtime coverage tools to ensure we were hitting as many legs of code
> as possible in drivers, and to cover these possibilities we need to do
> various different types of completion gathering, e.g. gather multiple
> bursts in one go, gathering a single burst in two halves, gathering a burst
> using completion_status rather than completion, gathering completions
> one-at-a-time with a call for each individually, and for error handling
> gathering just the failing element alone, or gathering completions for all
> remaining elements not just the failing one, etc. etc.
Agree with the rationale.
>
> These tests are useful both for finding bugs (and they did find ones in our
> drivers), but also to ensure similar behaviour across different drivers
> using the API. However, they really only can be done in a consistent way if
> we are able to ensure that at certain points the hardware has finished
> processing before we begin gathering completions. Therefore, having a way
> to poll for idle is useful. As you see, I've also left in the delay as a
> fallback in case drivers choose not to implement it, thereby making it an
> optional API.
>
> Beyond testing, I can see the API to poll for idleness being useful for the
> device shutdown case. I was considering whether the "stop" API should also
> use it to ensure that the hardware is idle before stopping. I decided
> against it for now, but you could see applications making use of this -
> waiting for the hardware to finish its work before stopping it.
I think 25us will not be enough, e.s.p If is PCI-Dev to PCI-Dev kind
of test cases.
Since it is the functional test case, I think, we can keep it a very
higher range to
support all cases. Maybe 50ms is a good target.
>
> Regards,
> /Bruce