Hi Declan,
> > //snip
> >
> >> nb_devs = rte_cryptodev_count();
> >> if (nb_devs < 1) {
> >> RTE_LOG(WARNING, USER1, "No crypto devices found?\n");
> >> @@ -838,6 +630,228 @@ testsuite_setup(void)
> >> return TEST_SUCCESS;
> >> }
> >>
> >> +static int
> >> +qat_testsuite_setup(void)
> >> +{
> >> + return testsuite_params_setup();
> >> +}
> >> +
> >> +static int
> >> +aesni_mb_testsuite_setup(void)
> >> +{
> >> + int32_t nb_devs, ret;
> >> + nb_devs = rte_cryptodev_device_count_by_driver(
> >> + rte_cryptodev_driver_id_get(
> >> + RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD)));
> >> + if (nb_devs < 1) {
> >> + ret =
> >> rte_vdev_init(RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD), NULL);
> >> +
> >> + TEST_ASSERT(ret == 0,
> >> + "Failed to create instance of pmd : %s",
> >> + RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD));
> >> + }
> >> + return testsuite_params_setup();
> >> +}
> >
> > Why is it required to have a separate function for each of the PMD?
> > I believe a single function with an argument should be sufficient.
> > And in that single function a simple switch case may process the vdevs
> differently.
> >
> > As of now, it looks that moving the unnecessary duplicate code from
> > one place to another.
> >
> > This was one cleanup we have been looking forward to from quite some
> time.
> > Every time a new PMD comes, a separate function is formed and the length
> of
> > the code increases.
> >
> >
> > //snip
>
> The virtual device creation code could be dropped completely if user is
> guaranteed to create the required crypto PMD from EAL devargs and we
> could assume that the first crypto device of the test suite type is
> always used?
Agreed.
>
> >>
> >> -static struct unit_test_suite cryptodev_virtio_testsuite = {
> >> +static struct unit_test_suite cryptodev_virtio_sub_testsuite = {
> >> .suite_name = "Crypto VIRTIO Unit Test Suite",
> >> - .setup = testsuite_setup,
> >> - .teardown = testsuite_teardown,
> >> .unit_test_cases = {
> >> TEST_CASE_ST(ut_setup, ut_teardown,
> >> test_AES_cipheronly_all),
> >>
> >> @@ -13935,15 +14107,12 @@ static struct unit_test_suite
> >> cryptodev_virtio_testsuite = {
> >> }
> >> };
> >>
> >> -static struct unit_test_suite cryptodev_caam_jr_testsuite = {
> >> - .suite_name = "Crypto CAAM JR Unit Test Suite",
> >> - .setup = testsuite_setup,
> >> - .teardown = testsuite_teardown,
> >> +static struct unit_test_suite cryptodev_caam_jr_sub_testsuite = {
> >> + .suite_name = "Crypto CAAM JR Sub Unit Test Suite",
> >> .unit_test_cases = {
> >> TEST_CASE_ST(ut_setup, ut_teardown,
> >> - test_device_configure_invalid_dev_id),
> >> - TEST_CASE_ST(ut_setup, ut_teardown,
> >> - test_multi_session),
> >> + test_device_configure_invalid_dev_id),
> >> + TEST_CASE_ST(ut_setup, ut_teardown, test_multi_session),
> >>
> >> TEST_CASE_ST(ut_setup, ut_teardown, test_AES_chain_all),
> >> TEST_CASE_ST(ut_setup, ut_teardown, test_3DES_chain_all),
> >> @@ -13955,58 +14124,28 @@ static struct unit_test_suite
> >> cryptodev_caam_jr_testsuite = {
> >> }
> >> };
> >
> > Splitting the complete testsuite into logical generic algo based sub
> > testsuite
> > Is a good idea. I appreciate that.
> >
> > But introducing PMD based test suite is not recommended. We have been
> > trying from past few releases to clean this up. And this patch is again
> introducing
> > the same. When I first saw this series, I saw only the algo based splitting
> and
> > when it was run on the board, it was showing results in an organized way.
> > But this was not expected that, PMD based test suites are reintroduced by
> > Intel who helped in removing them in last few releases.
> >
> > This will make an unnecessary addition of duplicate code whenever a new
> PMD
> > is introduced.
> >
> > I recommend to use a single parent suite - cryptodev_testsuite and there
> > Can be multiple sub testsuites based on Algos etc. but not on the basis of
> PMD.
> >
> > Regards,
> > Akhil
> >
>
> Hey Akhil, I understand the sentiment of this, we were just trying to
> avoid necessary failures by executing testsuites which aren't supported
> by the PMD under test, and we're confident that all testsuites/tests are
> correctly verifying their capabilities requirements. If we add some code
> into the testsuite setup functions to test capabilities required for the
> testsuites vs those required by the PMD then we could do as you are
> suggesting. If we can make this change quickly would you consider this
> patchset for inclusion in RC2?
I can take these patches upto RC2.
