[v4,0/7] test: refactor crypto unit test framework

  The current crypto unit test framework is not granular enough to
accurately track unit test results. This is caused by one testcase
in a suite actually running multiple testcases, but only returning
one result.
 
The approach taken in this patchset allows a test suite have a
list of sub-testsuites, and/or a list of testcases as previously used.
The unit test suite runner can then recursively iterate and run the
sub-testsuites, until it reaches a suite with testcases,
and it then runs each testcase as it had done previously.
In the case of a testsuite with both testcases and sub-testsuites,
the testcases are executed first before iterating through the
sub-testsuites.
 
By allowing this further breakdown into sub-testsuites,
a refactor of the crypto unit tests solves the issue of inaccurate
reporting, as sub-testsuites can be used in place of the testcases
that had multiple testcases hidden on a sub level.
The blockcipher tests previously had these hidden testcases,
but are now sub-testsuites that are dynamically created and added to a
parent test suite, allowing for each testcase status to be reported
directly to the runner.
The cryptodev test suite is broken down into smaller suites that are
used as sub-testsuites, which allows for more flexibility choosing which
sub-testsuites should run for the current device.
The introduction of sub-testsuites also allows for more precise
setup/teardown functions, that can check the capabilities required to
run its testcases.
 
For example, when running the cryptodev_aesni_mb_autotest, 
the parent Cryptodev Test Suite is executed.
Various sub-testsuites are added to the parent test suite, such as
the static suites of testcases that were once in the cryptodev_testsuite,
and blockcipher suites.
The unit test runner can then run the Cryptodev parent test suite,
which in turn will run the sub-testsuites.

The user is now required to create vdevs via EAL commandline args,
this is no longer done within the test app for crypto autotests.

Documentation will need to be added at a later stage,
adding to the test document that isn't yet merged. [1]

[1] https://patchwork.dpdk.org/project/dpdk/patch/20210309155757.615536-1-aconole@redhat.com/


v4:
  - Fixed some return value bugs related to patch 4.
  - Rebased onto main.
v3:
  - Added support for a testsuite having both a list of testcases,
    and a list of sub-testsuites.
  - Replaced PMD based parent testsuites with a cryptodev testsuite
    used by all autotests, with the exception of scheduler autotest.
  - Setup functions were added for all sub-testsuites, within which
    required capability support is checked.
  - The setup functions no longer create vdevs if needed,
    this must be done by the user when running the test.
  - Patch added to standardise return values for skipped testcases.
v2:
  - Added macro in place of testcase/testsuite loops.
  - Added more detail in the summary output.
  - Moved testcase counts to the testsuite structure.
  - Flattened testsuite structure to remove union.
  - Added patch for fix of blockcipher test return value.
  - Squashed release note into last patch.

Ciara Power (7):
  app/test: refactor of unit test suite runner
  test: introduce parent testsuite format
  test/crypto: refactor to use sub-testsuites
  test/crypto: replace unsupported with skipped
  test/crypto: move testsuite params to header file
  test/crypto: fix return value on test skipped
  test/crypto: dynamically build blockcipher suite

 app/test/test.c                        |  176 +-
 app/test/test.h                        |   23 +-
 app/test/test_cryptodev.c              | 2333 ++++++++++++++----------
 app/test/test_cryptodev.h              |   32 +
 app/test/test_cryptodev_asym.c         |  111 +-
 app/test/test_cryptodev_blockcipher.c  |  423 ++++-
 app/test/test_cryptodev_blockcipher.h  |   12 +-
 app/test/test_ipsec.c                  |   32 +-
 doc/guides/rel_notes/release_21_05.rst |    5 +
 9 files changed, 2004 insertions(+), 1143 deletions(-)
  

Hi Ciara,

I see that CI is reporting one error in autotest. Can you please check if it is because of your patchset?

9/96 DPDK:fast-tests / cycles_autotest                FAIL            1.42s   (exit status 255 or signal 127 SIGinvalid)
12:33:15 DPDK_TEST=cycles_autotest MALLOC_PERTURB_=44 /home/jenkins-local/jenkins-agent/workspace/Ubuntu18.04-Unit-Test-DPDK/dpdk/build/app/test/dpdk-test '-l 0-3' --file-prefix=cycles_autotest
----------------------------------- output -----------------------------------
stdout:
RTE>>cycles_autotest

delay_us is not accurate: too long
Test Failed


Regards,
Akhil

> -----Original Message-----
> From: Ciara Power <ciara.power@intel.com>
> Sent: Wednesday, May 12, 2021 5:07 PM
> To: dev@dpdk.org
> Cc: declan.doherty@intel.com; Akhil Goyal <gakhil@marvell.com>;
> aconole@redhat.com; hemant.agrawal@nxp.com; Anoob Joseph
> <anoobj@marvell.com>; ruifeng.wang@arm.com; asomalap@amd.com;
> ajit.khaparde@broadcom.com; g.singh@nxp.com; roy.fan.zhang@intel.com;
> Ciara Power <ciara.power@intel.com>
> Subject: [EXT] [PATCH v4 0/7] test: refactor crypto unit test framework
> 
> External Email
> 
> ----------------------------------------------------------------------
> The current crypto unit test framework is not granular enough to
> accurately track unit test results. This is caused by one testcase
> in a suite actually running multiple testcases, but only returning
> one result.
> 
> The approach taken in this patchset allows a test suite have a
> list of sub-testsuites, and/or a list of testcases as previously used.
> The unit test suite runner can then recursively iterate and run the
> sub-testsuites, until it reaches a suite with testcases,
> and it then runs each testcase as it had done previously.
> In the case of a testsuite with both testcases and sub-testsuites,
> the testcases are executed first before iterating through the
> sub-testsuites.
> 
> By allowing this further breakdown into sub-testsuites,
> a refactor of the crypto unit tests solves the issue of inaccurate
> reporting, as sub-testsuites can be used in place of the testcases
> that had multiple testcases hidden on a sub level.
> The blockcipher tests previously had these hidden testcases,
> but are now sub-testsuites that are dynamically created and added to a
> parent test suite, allowing for each testcase status to be reported
> directly to the runner.
> The cryptodev test suite is broken down into smaller suites that are
> used as sub-testsuites, which allows for more flexibility choosing which
> sub-testsuites should run for the current device.
> The introduction of sub-testsuites also allows for more precise
> setup/teardown functions, that can check the capabilities required to
> run its testcases.
> 
> For example, when running the cryptodev_aesni_mb_autotest,
> the parent Cryptodev Test Suite is executed.
> Various sub-testsuites are added to the parent test suite, such as
> the static suites of testcases that were once in the cryptodev_testsuite,
> and blockcipher suites.
> The unit test runner can then run the Cryptodev parent test suite,
> which in turn will run the sub-testsuites.
> 
> The user is now required to create vdevs via EAL commandline args,
> this is no longer done within the test app for crypto autotests.
> 
> Documentation will need to be added at a later stage,
> adding to the test document that isn't yet merged. [1]
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patchwork.dpdk.org_project_dpdk_patch_20210309155757.615536-
> 2D1-2Daconole-
> 40redhat.com_&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PR
> wpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=XItKx7UnOATOiVC3kRLlV3Woh
> rLeZbmlqShu_py452U&s=BpOMigsNnvWMV739-
> fMxEvVERJieNSnfnnmZdiMLMJg&e=
> 
> 
> v4:
>   - Fixed some return value bugs related to patch 4.
>   - Rebased onto main.
> v3:
>   - Added support for a testsuite having both a list of testcases,
>     and a list of sub-testsuites.
>   - Replaced PMD based parent testsuites with a cryptodev testsuite
>     used by all autotests, with the exception of scheduler autotest.
>   - Setup functions were added for all sub-testsuites, within which
>     required capability support is checked.
>   - The setup functions no longer create vdevs if needed,
>     this must be done by the user when running the test.
>   - Patch added to standardise return values for skipped testcases.
> v2:
>   - Added macro in place of testcase/testsuite loops.
>   - Added more detail in the summary output.
>   - Moved testcase counts to the testsuite structure.
>   - Flattened testsuite structure to remove union.
>   - Added patch for fix of blockcipher test return value.
>   - Squashed release note into last patch.
> 
> Ciara Power (7):
>   app/test: refactor of unit test suite runner
>   test: introduce parent testsuite format
>   test/crypto: refactor to use sub-testsuites
>   test/crypto: replace unsupported with skipped
>   test/crypto: move testsuite params to header file
>   test/crypto: fix return value on test skipped
>   test/crypto: dynamically build blockcipher suite
> 
>  app/test/test.c                        |  176 +-
>  app/test/test.h                        |   23 +-
>  app/test/test_cryptodev.c              | 2333 ++++++++++++++----------
>  app/test/test_cryptodev.h              |   32 +
>  app/test/test_cryptodev_asym.c         |  111 +-
>  app/test/test_cryptodev_blockcipher.c  |  423 ++++-
>  app/test/test_cryptodev_blockcipher.h  |   12 +-
>  app/test/test_ipsec.c                  |   32 +-
>  doc/guides/rel_notes/release_21_05.rst |    5 +
>  9 files changed, 2004 insertions(+), 1143 deletions(-)
> 
> --
> 2.25.1
  

I see this patchset is merged in the crypto tree but I guess it is not definitive.
Am I right it is too late to do such refactoring after -rc2?
What is the intent?


12/05/2021 15:15, Akhil Goyal:
> Hi Ciara,
> 
> I see that CI is reporting one error in autotest. Can you please check if it is because of your patchset?
> 
> 9/96 DPDK:fast-tests / cycles_autotest                FAIL            1.42s   (exit status 255 or signal 127 SIGinvalid)
> 12:33:15 DPDK_TEST=cycles_autotest MALLOC_PERTURB_=44 /home/jenkins-local/jenkins-agent/workspace/Ubuntu18.04-Unit-Test-DPDK/dpdk/build/app/test/dpdk-test '-l 0-3' --file-prefix=cycles_autotest
> ----------------------------------- output -----------------------------------
> stdout:
> RTE>>cycles_autotest
> 
> delay_us is not accurate: too long
> Test Failed
> 
> 
> Regards,
> Akhil
> 
> > -----Original Message-----
> > From: Ciara Power <ciara.power@intel.com>
> > Sent: Wednesday, May 12, 2021 5:07 PM
> > To: dev@dpdk.org
> > Cc: declan.doherty@intel.com; Akhil Goyal <gakhil@marvell.com>;
> > aconole@redhat.com; hemant.agrawal@nxp.com; Anoob Joseph
> > <anoobj@marvell.com>; ruifeng.wang@arm.com; asomalap@amd.com;
> > ajit.khaparde@broadcom.com; g.singh@nxp.com; roy.fan.zhang@intel.com;
> > Ciara Power <ciara.power@intel.com>
> > Subject: [EXT] [PATCH v4 0/7] test: refactor crypto unit test framework
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > The current crypto unit test framework is not granular enough to
> > accurately track unit test results. This is caused by one testcase
> > in a suite actually running multiple testcases, but only returning
> > one result.
> > 
> > The approach taken in this patchset allows a test suite have a
> > list of sub-testsuites, and/or a list of testcases as previously used.
> > The unit test suite runner can then recursively iterate and run the
> > sub-testsuites, until it reaches a suite with testcases,
> > and it then runs each testcase as it had done previously.
> > In the case of a testsuite with both testcases and sub-testsuites,
> > the testcases are executed first before iterating through the
> > sub-testsuites.
> > 
> > By allowing this further breakdown into sub-testsuites,
> > a refactor of the crypto unit tests solves the issue of inaccurate
> > reporting, as sub-testsuites can be used in place of the testcases
> > that had multiple testcases hidden on a sub level.
> > The blockcipher tests previously had these hidden testcases,
> > but are now sub-testsuites that are dynamically created and added to a
> > parent test suite, allowing for each testcase status to be reported
> > directly to the runner.
> > The cryptodev test suite is broken down into smaller suites that are
> > used as sub-testsuites, which allows for more flexibility choosing which
> > sub-testsuites should run for the current device.
> > The introduction of sub-testsuites also allows for more precise
> > setup/teardown functions, that can check the capabilities required to
> > run its testcases.
> > 
> > For example, when running the cryptodev_aesni_mb_autotest,
> > the parent Cryptodev Test Suite is executed.
> > Various sub-testsuites are added to the parent test suite, such as
> > the static suites of testcases that were once in the cryptodev_testsuite,
> > and blockcipher suites.
> > The unit test runner can then run the Cryptodev parent test suite,
> > which in turn will run the sub-testsuites.
> > 
> > The user is now required to create vdevs via EAL commandline args,
> > this is no longer done within the test app for crypto autotests.
> > 
> > Documentation will need to be added at a later stage,
> > adding to the test document that isn't yet merged. [1]
> > 
> > [1] https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__patchwork.dpdk.org_project_dpdk_patch_20210309155757.615536-
> > 2D1-2Daconole-
> > 40redhat.com_&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PR
> > wpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=XItKx7UnOATOiVC3kRLlV3Woh
> > rLeZbmlqShu_py452U&s=BpOMigsNnvWMV739-
> > fMxEvVERJieNSnfnnmZdiMLMJg&e=
> > 
> > 
> > v4:
> >   - Fixed some return value bugs related to patch 4.
> >   - Rebased onto main.
> > v3:
> >   - Added support for a testsuite having both a list of testcases,
> >     and a list of sub-testsuites.
> >   - Replaced PMD based parent testsuites with a cryptodev testsuite
> >     used by all autotests, with the exception of scheduler autotest.
> >   - Setup functions were added for all sub-testsuites, within which
> >     required capability support is checked.
> >   - The setup functions no longer create vdevs if needed,
> >     this must be done by the user when running the test.
> >   - Patch added to standardise return values for skipped testcases.
> > v2:
> >   - Added macro in place of testcase/testsuite loops.
> >   - Added more detail in the summary output.
> >   - Moved testcase counts to the testsuite structure.
> >   - Flattened testsuite structure to remove union.
> >   - Added patch for fix of blockcipher test return value.
> >   - Squashed release note into last patch.
> > 
> > Ciara Power (7):
> >   app/test: refactor of unit test suite runner
> >   test: introduce parent testsuite format
> >   test/crypto: refactor to use sub-testsuites
> >   test/crypto: replace unsupported with skipped
> >   test/crypto: move testsuite params to header file
> >   test/crypto: fix return value on test skipped
> >   test/crypto: dynamically build blockcipher suite
> > 
> >  app/test/test.c                        |  176 +-
> >  app/test/test.h                        |   23 +-
> >  app/test/test_cryptodev.c              | 2333 ++++++++++++++----------
> >  app/test/test_cryptodev.h              |   32 +
> >  app/test/test_cryptodev_asym.c         |  111 +-
> >  app/test/test_cryptodev_blockcipher.c  |  423 ++++-
> >  app/test/test_cryptodev_blockcipher.h  |   12 +-
> >  app/test/test_ipsec.c                  |   32 +-
> >  doc/guides/rel_notes/release_21_05.rst |    5 +
> >  9 files changed, 2004 insertions(+), 1143 deletions(-)
> > 
> > --
> > 2.25.1
> 
>
  

> I see this patchset is merged in the crypto tree but I guess it is not definitive.
> Am I right it is too late to do such refactoring after -rc2?
> What is the intent?
> 
The case is not failing in my testing. Neither it is reproducible at Ciara's end.
Ciara has asked to rerun the CI to check if it is false positive.
I don't want to defer this series as it passes all test except the CI and has been
Reviewed by multiple people.
I am merging this patch for RC2. If there is any issue, it can be fixed in RC3

Regards,
Akhil
  

12/05/2021 18:00, Akhil Goyal:
> > I see this patchset is merged in the crypto tree but I guess it is not definitive.
> > Am I right it is too late to do such refactoring after -rc2?
> > What is the intent?
> > 
> The case is not failing in my testing. Neither it is reproducible at Ciara's end.
> Ciara has asked to rerun the CI to check if it is false positive.
> I don't want to defer this series as it passes all test except the CI and has been
> Reviewed by multiple people.
> I am merging this patch for RC2. If there is any issue, it can be fixed in RC3

-rc2 was closed last week. I am supposed to close -rc3 today.
Do you confirm it is OK to pull today?
  

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, May 12, 2021 9:35 PM
> To: Ciara Power <ciara.power@intel.com>; Akhil Goyal
> <gakhil@marvell.com>
> Cc: dev@dpdk.org; declan.doherty@intel.com; aconole@redhat.com;
> hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>;
> ruifeng.wang@arm.com; asomalap@amd.com;
> ajit.khaparde@broadcom.com; g.singh@nxp.com; roy.fan.zhang@intel.com
> Subject: Re: [dpdk-dev] [EXT] [PATCH v4 0/7] test: refactor crypto unit test
> framework
> 
> 12/05/2021 18:00, Akhil Goyal:
> > > I see this patchset is merged in the crypto tree but I guess it is not
> definitive.
> > > Am I right it is too late to do such refactoring after -rc2?
> > > What is the intent?
> > >
> > The case is not failing in my testing. Neither it is reproducible at Ciara's end.
> > Ciara has asked to rerun the CI to check if it is false positive.
> > I don't want to defer this series as it passes all test except the CI and has
> been
> > Reviewed by multiple people.
> > I am merging this patch for RC2. If there is any issue, it can be fixed in RC3
> 
> -rc2 was closed last week. I am supposed to close -rc3 today.
> Do you confirm it is OK to pull today?
> 
Yes you can pull in 30 mins.
I meant RC3 . 😊
  

Akhil Goyal <gakhil@marvell.com> writes:

> Hi Ciara,
>
> I see that CI is reporting one error in autotest. Can you please check
> if it is because of your patchset?
>
> 9/96 DPDK:fast-tests / cycles_autotest FAIL 1.42s (exit status 255 or
> signal 127 SIGinvalid)

Just to note, I'm looking into that test case a bit more.  It would be
nice if the failures were asserts that listed the values.  I also wonder
if it's possible that we hit some kind of delay on wake up (especially
on busy systems like CI systems) and should have a flag to disable
this.  I can see how it's possible for this test case to give a false
positive in some system configurations but not sure how best to remedy
that situation.

> 12:33:15 DPDK_TEST=cycles_autotest MALLOC_PERTURB_=44
> /home/jenkins-local/jenkins-agent/workspace/Ubuntu18.04-Unit-Test-DPDK/dpdk/build/app/test/dpdk-test
> '-l 0-3' --file-prefix=cycles_autotest
> ----------------------------------- output -----------------------------------
> stdout:
> RTE>>cycles_autotest
>
> delay_us is not accurate: too long
> Test Failed
>
>
> Regards,
> Akhil
>
>> -----Original Message-----
>> From: Ciara Power <ciara.power@intel.com>
>> Sent: Wednesday, May 12, 2021 5:07 PM
>> To: dev@dpdk.org
>> Cc: declan.doherty@intel.com; Akhil Goyal <gakhil@marvell.com>;
>> aconole@redhat.com; hemant.agrawal@nxp.com; Anoob Joseph
>> <anoobj@marvell.com>; ruifeng.wang@arm.com; asomalap@amd.com;
>> ajit.khaparde@broadcom.com; g.singh@nxp.com; roy.fan.zhang@intel.com;
>> Ciara Power <ciara.power@intel.com>
>> Subject: [EXT] [PATCH v4 0/7] test: refactor crypto unit test framework
>> 
>> External Email
>> 
>> ----------------------------------------------------------------------
>> The current crypto unit test framework is not granular enough to
>> accurately track unit test results. This is caused by one testcase
>> in a suite actually running multiple testcases, but only returning
>> one result.
>> 
>> The approach taken in this patchset allows a test suite have a
>> list of sub-testsuites, and/or a list of testcases as previously used.
>> The unit test suite runner can then recursively iterate and run the
>> sub-testsuites, until it reaches a suite with testcases,
>> and it then runs each testcase as it had done previously.
>> In the case of a testsuite with both testcases and sub-testsuites,
>> the testcases are executed first before iterating through the
>> sub-testsuites.
>> 
>> By allowing this further breakdown into sub-testsuites,
>> a refactor of the crypto unit tests solves the issue of inaccurate
>> reporting, as sub-testsuites can be used in place of the testcases
>> that had multiple testcases hidden on a sub level.
>> The blockcipher tests previously had these hidden testcases,
>> but are now sub-testsuites that are dynamically created and added to a
>> parent test suite, allowing for each testcase status to be reported
>> directly to the runner.
>> The cryptodev test suite is broken down into smaller suites that are
>> used as sub-testsuites, which allows for more flexibility choosing which
>> sub-testsuites should run for the current device.
>> The introduction of sub-testsuites also allows for more precise
>> setup/teardown functions, that can check the capabilities required to
>> run its testcases.
>> 
>> For example, when running the cryptodev_aesni_mb_autotest,
>> the parent Cryptodev Test Suite is executed.
>> Various sub-testsuites are added to the parent test suite, such as
>> the static suites of testcases that were once in the cryptodev_testsuite,
>> and blockcipher suites.
>> The unit test runner can then run the Cryptodev parent test suite,
>> which in turn will run the sub-testsuites.
>> 
>> The user is now required to create vdevs via EAL commandline args,
>> this is no longer done within the test app for crypto autotests.
>> 
>> Documentation will need to be added at a later stage,
>> adding to the test document that isn't yet merged. [1]
>> 
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__patchwork.dpdk.org_project_dpdk_patch_20210309155757.615536-
>> 2D1-2Daconole-
>> 40redhat.com_&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PR
>> wpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=XItKx7UnOATOiVC3kRLlV3Woh
>> rLeZbmlqShu_py452U&s=BpOMigsNnvWMV739-
>> fMxEvVERJieNSnfnnmZdiMLMJg&e=
>> 
>> 
>> v4:
>>   - Fixed some return value bugs related to patch 4.
>>   - Rebased onto main.
>> v3:
>>   - Added support for a testsuite having both a list of testcases,
>>     and a list of sub-testsuites.
>>   - Replaced PMD based parent testsuites with a cryptodev testsuite
>>     used by all autotests, with the exception of scheduler autotest.
>>   - Setup functions were added for all sub-testsuites, within which
>>     required capability support is checked.
>>   - The setup functions no longer create vdevs if needed,
>>     this must be done by the user when running the test.
>>   - Patch added to standardise return values for skipped testcases.
>> v2:
>>   - Added macro in place of testcase/testsuite loops.
>>   - Added more detail in the summary output.
>>   - Moved testcase counts to the testsuite structure.
>>   - Flattened testsuite structure to remove union.
>>   - Added patch for fix of blockcipher test return value.
>>   - Squashed release note into last patch.
>> 
>> Ciara Power (7):
>>   app/test: refactor of unit test suite runner
>>   test: introduce parent testsuite format
>>   test/crypto: refactor to use sub-testsuites
>>   test/crypto: replace unsupported with skipped
>>   test/crypto: move testsuite params to header file
>>   test/crypto: fix return value on test skipped
>>   test/crypto: dynamically build blockcipher suite
>> 
>>  app/test/test.c                        |  176 +-
>>  app/test/test.h                        |   23 +-
>>  app/test/test_cryptodev.c              | 2333 ++++++++++++++----------
>>  app/test/test_cryptodev.h              |   32 +
>>  app/test/test_cryptodev_asym.c         |  111 +-
>>  app/test/test_cryptodev_blockcipher.c  |  423 ++++-
>>  app/test/test_cryptodev_blockcipher.h  |   12 +-
>>  app/test/test_ipsec.c                  |   32 +-
>>  doc/guides/rel_notes/release_21_05.rst |    5 +
>>  9 files changed, 2004 insertions(+), 1143 deletions(-)
>> 
>> --
>> 2.25.1