[dpdk-dev] [PATCH] doc: add meson ut enhancements in prog guide

[dpdk-dev] [PATCH] doc: add meson ut enhancements in prog guide

[Hari Kumar Vemula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6296 bytes --]

Add a programmer's guide section for meson ut enhancements

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
 doc/guides/prog_guide/index.rst               |   1 +
 .../prog_guide/meson_ut_enhancements.rst      | 158 ++++++++++++++++++
 2 files changed, 159 insertions(+)
 create mode 100644 doc/guides/prog_guide/meson_ut_enhancements.rst

diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index ba8c1f6ad..41971e1cf 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -57,6 +57,7 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    meson_ut_enhancements
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut_enhancements.rst b/doc/guides/prog_guide/meson_ut_enhancements.rst
new file mode 100644
index 000000000..328adda28
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut_enhancements.rst
@@ -0,0 +1,158 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+    Copyright(c) 2014-2018 Intel Corporation.
+
+.. _Meson_UT_Enhancements:
+
+Meson_UT_Enhancements
+=====================
+
+The meson build support for unit tests is now available and the more enhancements are done to the build system
+to classify unit tests under different categories. The build `test/test/meson.build` file has been
+modified for these enhancements. This document will further down describe the below list.
+
+*  Building and Running the unit tests.
+*  Grouping of testcases.
+*  Parallel and non parallel tests.
+*  Test suites.
+*  How to run different test suites.
+*  Support for skipped tests.
+
+
+Building and Running the unit tests
+-----------------------------------
+
+*  Create the meson build output folder using command.
+
+   ``$meson <build_dir>``
+
+*  Enter into build output folder, which was created by above command.
+
+   ``$cd build``
+
+*  Compile DPDK using `$ninja`.
+   The output file of the build will be available in meson build folder.
+   After successful ninja, binary `dpdk-test` is created in `build/test/test/`.
+
+*  Run the unit testcases.
+
+   ``$ninja test`` (or) ``$meson test``
+
+*  To rebuild the build directory, after any changes to meson.build.
+
+   ``$meson configure``
+
+*   To run specific test case via meson command.
+
+   ``$meson test <test case>`` (or) ``$ninja test <test case>``
+
+
+Grouping of testcases
+---------------------
+
+Testcases has been grouped into below four different groups based on conditions
+of time duration and performance of the individual testcase.
+
+*  fast_parallel_test_names
+*  fast_non_parallel_test_names
+*  perf_test_names
+*  driver_test_names
+*  dump_test_names
+
+Parallel and non parallel tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Unless specified the meson will run all unit tests as parallel by default.
+So the test cases are categorized into parallel and non parallel tests purely
+based test case functionality using `is_parallel` argument of `test()`
+in meson.build. Test cases marked with `is_parallel : true` will run in
+parallel and tests marked with `is_parallel : false` will run in non-parallel.
+While non-parallel test is running, any other test should not be run.
+Parallel and non parallel test cases are grouped under the
+`fast_parallel_test_names` and `fast_non_parallel_test_names`.
+
+Test suites
+~~~~~~~~~~~
+
+Test groups are considered as "suite” in `meson.build` and can be provided
+as argument to `test()` as  `suite ‘project_name:label’`
+
+    Ex: ``suite ‘DPDK:fast-tests’``
+
+Running different test suites
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Below are commands to run testcases using option `suite`
+
+*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`
+   are run under 10seconds and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:fast-tests``
+
+*  Test cases from the group `perf_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:perf-tests``
+
+*  Test cases from the group `driver_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:driver-tests``
+
+*  Test cases from the group `dump_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:dump-tests``
+
+
+Skipped testcases
+-----------------
+
+Some unit test cases have dependency on external libraries, driver modules or
+config flags, without which the test cases cannot be run, such test cases
+should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
+test cases run user should enable relevant dependencies. Below are the few
+current scenarios when test cases are skipped:
+
+#. External library dependency paths are not set.
+#. Config flag for the dependent library is not enabled.
+#. Dependent driver modules are not installed on the system.
+
+Dependent library paths can be set using below
+
+*  Single path ``export LIBRARY_PATH=path``
+
+*  Multiple paths ``export LIBRARY_PATH=path1:path2:path3``
+
+Dependent library headers path can be exported as below.
+
+*  Single path ``$CFLAGS=-I/path meson build``
+
+*  Multiple paths ``$CFLAGS=-I/path1 -I/path2 meson build``
+
+Below examples shows how to export libraries and their header paths.
+
+To export single library at a time.
+
+        ``$export LIBRARY_PATH=/root/wireless_libs/zuc/``
+        ``$CFLAGS=-I/root/wireless_libs/zuc/include meson build``
+
+To export multiple libraries at a time.
+
+        ``$export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
+                                                    ``libsso_kasumi/build/``
+        ``$CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
+                                        ``libsso_kasumi/include meson build``
+
+
+
+Commands to run meson UTs
+-------------------------
+
+*   To run all test cases
+       ``$meson test``
+*   To run specific test
+       ``$meson test testcase_name``
+        Ex:``$meson test acl_autotest``
+*   To run specific test suite
+       ``$meson test --suite DPDK:suite_name``
+        Ex:``$meson test --suite DPDK:fast-tests``
-- 
2.17.2

Re: [dpdk-dev] [PATCH] doc: add meson ut enhancements in prog guide

[Thomas Monjalon]

12/12/2018 12:35, Hari Kumar Vemula:
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut_enhancements.rst
> @@ -0,0 +1,158 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +    Copyright(c) 2014-2018 Intel Corporation.
> +
> +.. _Meson_UT_Enhancements:

No need to add such anchor. The doc can be referred with :doc: syntax.

> +
> +Meson_UT_Enhancements
> +=====================

You should make a doc about how to run the tests with meson,
not how you improved it.
In short, please remove "enhancements" point of view.

> +
> +The meson build support for unit tests is now available and the more enhancements are done to the build system
> +to classify unit tests under different categories. The build `test/test/meson.build` file has been
> +modified for these enhancements. This document will further down describe the below list.

[dpdk-dev] [PATCH v2] doc: add meson ut info in prog guide

[Hari Kumar Vemula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6077 bytes --]

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
v2: Removed enhancement details
---
 doc/guides/prog_guide/index.rst    |   1 +
 doc/guides/prog_guide/meson_ut.rst | 164 +++++++++++++++++++++++++++++
 2 files changed, 165 insertions(+)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 6726b1e8d..f4274573f 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -58,6 +58,7 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..ab4adbbe8
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,164 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+
+    Copyright(c) 2014-2018 Intel Corporation.
+
+.. _Meson:
+
+Meson_UT
+========
+
+The meson build for unit tests under different categories is supported using 'test/test/meson.build'.
+
+This document describes the below list in detail.
+
+*  Building and Running the unit tests.
+*  Grouping of testcases.
+*  Parallel and non parallel tests.
+*  Test suites.
+*  How to run different test suites.
+*  Support for skipped tests.
+
+
+
+Building and Running the unit tests
+-----------------------------------
+
+*  Create the meson build output folder using command.
+
+   ``$meson <build_dir>``
+
+*  Enter into build output folder, which was created by above command.
+
+   ``$cd build``
+
+*  Compile DPDK using `$ninja`.
+   The output file of the build will be available in meson build folder.
+   After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
+
+*  Run the unit testcases.
+
+   ``$ninja test`` (or) ``$meson test``
+
+*  To rebuild the build directory, after any changes to meson.build.
+
+   ``$meson configure``
+
+*   To run specific test case via meson command.
+
+   ``$meson test <test case>`` (or) ``$ninja test <test case>``
+
+
+
+Grouping of testcases
+---------------------
+
+Testcases has been grouped into below four different groups based on conditions
+of time duration and performance of the individual testcase.
+
+*  fast_parallel_test_names
+*  fast_non_parallel_test_names
+*  perf_test_names
+*  driver_test_names
+*  dump_test_names
+
+Parallel and non parallel tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Unless specified the meson will run all unit tests as parallel by default.
+So the test cases are categorized into parallel and non parallel tests purely
+based on test case functionality using `is_parallel` argument of `test()`
+in meson.build. Test cases marked with `is_parallel : true` will run in
+parallel and tests marked with `is_parallel : false` will run in non-parallel.
+While non-parallel test is running, any other test should not be run.
+Parallel and non parallel test cases are grouped under the
+`fast_parallel_test_names` and `fast_non_parallel_test_names`.
+
+
+
+Test suites
+~~~~~~~~~~~
+
+Test groups are considered as "suite” in `meson.build` and can be provided
+as argument to `test()` as  `suite ‘project_name:label’`
+
+    Ex: ``suite ‘DPDK:fast-tests’``
+
+Running different test suites
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Below are commands to run testcases using option `suite`
+
+*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`
+   are run under 10seconds and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:fast-tests``
+
+*  Test cases from the group `perf_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:perf-tests``
+
+*  Test cases from the group `driver_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:driver-tests``
+
+*  Test cases from the group `dump_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$meson test --suite DPDK:dump-tests``
+
+
+
+Skipped testcases
+-----------------
+
+Some unit test cases have dependency on external libraries, driver modules or
+config flags, without which the test cases cannot be run, such test cases
+should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
+test cases run user should enable relevant dependencies. Below are the few
+current scenarios when test cases are skipped:
+
+#. External library dependency paths are not set.
+#. Config flag for the dependent library is not enabled.
+#. Dependent driver modules are not installed on the system.
+
+Dependent library paths can be set using below
+
+*  Single path ``export LIBRARY_PATH=path``
+
+*  Multiple paths ``export LIBRARY_PATH=path1:path2:path3``
+
+Dependent library headers path can be exported as below.
+
+*  Single path ``$CFLAGS=-I/path meson build``
+
+*  Multiple paths ``$CFLAGS=-I/path1 -I/path2 meson build``
+
+Below examples shows how to export libraries and their header paths.
+
+To export single library at a time.
+
+        ``$export LIBRARY_PATH=/root/wireless_libs/zuc/``
+        ``$CFLAGS=-I/root/wireless_libs/zuc/include meson build``
+
+To export multiple libraries at a time.
+
+        ``$export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
+                                                    ``libsso_kasumi/build/``
+        ``$CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
+                                        ``libsso_kasumi/include meson build``
+
+
+
+Commands to run meson UTs
+-------------------------
+
+*   To run all test cases
+       ``$meson test``
+*   To run specific test
+       ``$meson test testcase_name``
+        Ex:``$meson test acl_autotest``
+*   To run specific test suite
+       ``$meson test --suite DPDK:suite_name``
+        Ex:``$meson test --suite DPDK:fast-tests``
-- 
2.17.2

Re: [dpdk-dev] [PATCH v2] doc: add meson ut info in prog guide

[Bruce Richardson]

On Wed, Jan 23, 2019 at 06:37:07AM +0000, Hari Kumar Vemula wrote:
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
> v2: Removed enhancement details
> ---
>  doc/guides/prog_guide/index.rst    |   1 +
>  doc/guides/prog_guide/meson_ut.rst | 164 +++++++++++++++++++++++++++++
>  2 files changed, 165 insertions(+)
>  create mode 100644 doc/guides/prog_guide/meson_ut.rst
> 
> diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
> index 6726b1e8d..f4274573f 100644
> --- a/doc/guides/prog_guide/index.rst
> +++ b/doc/guides/prog_guide/index.rst
> @@ -58,6 +58,7 @@ Programmer's Guide
>      source_org
>      dev_kit_build_system
>      dev_kit_root_make_help
> +    meson_ut
>      extend_dpdk
>      build_app
>      ext_app_lib_make_help
> diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
> new file mode 100644
> index 000000000..ab4adbbe8
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,164 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +
> +    Copyright(c) 2014-2018 Intel Corporation.
> +
> +.. _Meson:
> +
> +Meson_UT
> +========
> +
> +The meson build for unit tests under different categories is supported using 'test/test/meson.build'.
> +
> +This document describes the below list in detail.
> +
> +*  Building and Running the unit tests.
> +*  Grouping of testcases.
> +*  Parallel and non parallel tests.
> +*  Test suites.
> +*  How to run different test suites.
> +*  Support for skipped tests.
> +
> +
> +
> +Building and Running the unit tests
> +-----------------------------------
> +
> +*  Create the meson build output folder using command.
> +
> +   ``$meson <build_dir>``

Here and with the commands below, leave a space between the '$' and the
command to show that it's the prompt and not the variable '$meson'

> +
> +*  Enter into build output folder, which was created by above command.
> +
> +   ``$cd build``
> +
> +*  Compile DPDK using `$ninja`.
> +   The output file of the build will be available in meson build folder.
> +   After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
> +
> +*  Run the unit testcases.
> +
> +   ``$ninja test`` (or) ``$meson test``
> +
> +*  To rebuild the build directory, after any changes to meson.build.
> +
> +   ``$meson configure``
> +

This should not be necesary. If you make any changes to meson.build then
ninja will automatically detect that and call meson to reconfigure itself
automatically.

Also, I don't think the rebuilding of the software is within the scope of
the section of the document. I'd omit the whole bullet point.

> +*   To run specific test case via meson command.
> +
> +   ``$meson test <test case>`` (or) ``$ninja test <test case>``
> +
> +
> +
> +Grouping of testcases
> +---------------------
> +
> +Testcases has been grouped into below four different groups based on conditions
> +of time duration and performance of the individual testcase.
> +
> +*  fast_parallel_test_names
> +*  fast_non_parallel_test_names
> +*  perf_test_names
> +*  driver_test_names
> +*  dump_test_names
> +
> +Parallel and non parallel tests
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Unless specified the meson will run all unit tests as parallel by default.

"meson" rather than "the meson" sounds better. Comma needed after
"specified". I'd also look to shorten the next sentence and merge it with
the one above, which sounds incomplete.

> +So the test cases are categorized into parallel and non parallel tests purely
> +based on test case functionality using `is_parallel` argument of `test()`
> +in meson.build. Test cases marked with `is_parallel : true` will run in
> +parallel and tests marked with `is_parallel : false` will run in non-parallel.
> +While non-parallel test is running, any other test should not be run.

"any other test should not be run" -> "no other tests should run".

> +Parallel and non parallel test cases are grouped under the
> +`fast_parallel_test_names` and `fast_non_parallel_test_names`.
> +
> +
> +
> +Test suites
> +~~~~~~~~~~~
> +
> +Test groups are considered as "suite??? in `meson.build` and can be provided
> +as argument to `test()` as  `suite ???project_name:label???`
> +
> +    Ex: ``suite ???DPDK:fast-tests???``

Watch out for the use of smart-quotes in the text above. Regular single and
double quotes are recommended.

When passing a suite to the meson to run, you use "--suite", so please
include the "--" prefix to avoid confusion.

> +
> +Running different test suites
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Below are commands to run testcases using option `suite`
> +
> +*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`
> +   are run under 10seconds and below is the meson command to run them.
> +
> +   ``$meson test --suite DPDK:fast-tests``
> +
> +*  Test cases from the group `perf_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$meson test --suite DPDK:perf-tests``
> +
> +*  Test cases from the group `driver_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$meson test --suite DPDK:driver-tests``
> +
> +*  Test cases from the group `dump_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$meson test --suite DPDK:dump-tests``
> +
> +
> +
> +Skipped testcases
> +-----------------
> +
> +Some unit test cases have dependency on external libraries, driver modules or
> +config flags, without which the test cases cannot be run, such test cases

Break the sentence after "run".

> +should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
> +test cases run user should enable relevant dependencies. Below are the few
> +current scenarios when test cases are skipped:
> +
> +#. External library dependency paths are not set.
> +#. Config flag for the dependent library is not enabled.
> +#. Dependent driver modules are not installed on the system.
> +
> +Dependent library paths can be set using below
> +
> +*  Single path ``export LIBRARY_PATH=path``
> +
> +*  Multiple paths ``export LIBRARY_PATH=path1:path2:path3``
> +
> +Dependent library headers path can be exported as below.
> +
> +*  Single path ``$CFLAGS=-I/path meson build``
> +
> +*  Multiple paths ``$CFLAGS=-I/path1 -I/path2 meson build``
> +

Please be consistent with use of "export" or not. Also, consistently (or
not) using "$" for prompt would be good too.

> +Below examples shows how to export libraries and their header paths.
> +
> +To export single library at a time.
> +
> +        ``$export LIBRARY_PATH=/root/wireless_libs/zuc/``
> +        ``$CFLAGS=-I/root/wireless_libs/zuc/include meson build``
> +
> +To export multiple libraries at a time.
> +
> +        ``$export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
> +                                                    ``libsso_kasumi/build/``
> +        ``$CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
> +                                        ``libsso_kasumi/include meson build``
> +
> +
> +
> +Commands to run meson UTs
> +-------------------------
> +

I'd suggest adding "Summary" into the title here - "Summary of Commands ..."

> +*   To run all test cases
> +       ``$meson test``
> +*   To run specific test
> +       ``$meson test testcase_name``
> +        Ex:``$meson test acl_autotest``
> +*   To run specific test suite
> +       ``$meson test --suite DPDK:suite_name``
> +        Ex:``$meson test --suite DPDK:fast-tests``
> -- 
> 2.17.2
>
Regards,
/Bruce 

[dpdk-dev] [PATCH v3] doc: add meson ut info in prog guide

[Hari Kumar Vemula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6031 bytes --]

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
v3: Modified
v2: Removed enhancement details
---
 doc/guides/prog_guide/index.rst    |   1 +
 doc/guides/prog_guide/meson_ut.rst | 159 +++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 6726b1e8d..f4274573f 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -58,6 +58,7 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..0e59f5526
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,159 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+
+    Copyright(c) 2014-2018 Intel Corporation.
+
+.. _Meson:
+
+Meson_UT
+========
+
+The meson build for unit tests under different categories is supported using 'test/test/meson.build'.
+
+This document describes the below list in detail.
+
+*  Building and Running the unit tests.
+*  Grouping of testcases.
+*  Parallel and non parallel tests.
+*  Test suites.
+*  How to run different test suites.
+*  Support for skipped tests.
+
+
+
+Building and Running the unit tests
+-----------------------------------
+
+*  Create the meson build output folder using command.
+
+   ``$ meson <build_dir>``
+
+*  Enter into build output folder, which was created by above command.
+
+   ``$ cd build``
+
+*  Compile DPDK using `$ ninja`.
+   The output file of the build will be available in meson build folder.
+   After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
+
+*  Run the unit testcases.
+
+   ``$ ninja test`` (or) ``$ meson test``
+
+*   To run specific test case via meson command.
+
+   ``$ meson test <test case>`` (or) ``$ ninja test <test case>``
+
+
+
+Grouping of testcases
+---------------------
+
+Testcases has been grouped into below four different groups based on conditions
+of time duration and performance of the individual testcase.
+
+*  fast_parallel_test_names
+*  fast_non_parallel_test_names
+*  perf_test_names
+*  driver_test_names
+*  dump_test_names
+
+Parallel and non parallel tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+By default, meson will run test cases in parallel. However `is_parallel` argument
+of `test()` in meson.build can be used to run tests in parallel or non-parallel
+mode based on its functionality. Test cases marked with `is_parallel : true` will
+run in parallel and tests marked with `is_parallel : false` will run in non-parallel.
+While non-parallel test is running, no other test should run.
+Parallel and non parallel test cases are grouped under the `fast_parallel_test_names`
+and `fast_non_parallel_test_names`.
+
+
+
+Test suites
+~~~~~~~~~~~
+
+Test groups are considered as `--suite` in `meson.build` and can be provided
+as argument to `test()` as  `--suite ‘project_name:label’`.
+
+    Ex: ``$ mesin test --suite ‘DPDK:fast-tests’``
+
+Running different test suites
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Below are commands to run testcases using option `suite`
+
+*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`
+   are run under 10seconds and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:fast-tests``
+
+*  Test cases from the group `perf_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:perf-tests``
+
+*  Test cases from the group `driver_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:driver-tests``
+
+*  Test cases from the group `dump_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:dump-tests``
+
+
+
+Skipped testcases
+-----------------
+
+Some unit test cases have dependency on external libraries, driver modules or
+config flags, without which the test cases cannot be run. such test cases
+should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
+test cases run user should enable relevant dependencies. Below are the few
+current scenarios when test cases are skipped:
+
+#. External library dependency paths are not set.
+#. Config flag for the dependent library is not enabled.
+#. Dependent driver modules are not installed on the system.
+
+Dependent library paths can be set using below
+
+*  Single path ``$ export LIBRARY_PATH=path``
+
+*  Multiple paths ``$ export LIBRARY_PATH=path1:path2:path3``
+
+Dependent library headers path can be stated as part of meson build command as below.
+
+*  Single path ``$ CFLAGS=-I/path meson build``
+
+*  Multiple paths ``$ CFLAGS=-I/path1 -I/path2 meson build``
+
+Below examples shows how to export libraries and their header paths.
+
+To specify single library at a time.
+
+        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/``
+        ``$ CFLAGS=-I/root/wireless_libs/zuc/include meson build``
+
+To specify multiple libraries at a time.
+
+        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
+                                                    ``libsso_kasumi/build/``
+        ``$ CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
+                                        ``libsso_kasumi/include meson build``
+
+
+
+Summery of Commands to run meson UTs
+------------------------------------
+
+*   To run all test cases
+       ``$ meson test``
+*   To run specific test
+       ``$ meson test testcase_name``
+        Ex:``$ meson test acl_autotest``
+*   To run specific test suite
+       ``$ meson test --suite DPDK:suite_name``
+        Ex:``$ meson test --suite DPDK:fast-tests``
-- 
2.17.2

Re: [dpdk-dev] [PATCH v3] doc: add meson ut info in prog guide

[Richardson, Bruce]

> -----Original Message-----
> From: Vemula, Hari KumarX
> Sent: Thursday, January 24, 2019 1:42 PM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>; Parthasarathy, JananeeX M
> <jananeex.m.parthasarathy@intel.com>; Vemula, Hari KumarX
> <hari.kumarx.vemula@intel.com>
> Subject: [PATCH v3] doc: add meson ut info in prog guide
> 
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
> v3: Modified
> v2: Removed enhancement details
> ---

Some typos and suggested reworking below.

/Bruce

<snip>

> +
> +
> +Test suites
> +~~~~~~~~~~~
> +
> +Test groups are considered as `--suite` in `meson.build` and can be

"Tests are grouped into test suites in meson.build and these suite names can be"

> +provided as argument to `test()` as  `--suite ‘project_name:label’`.

"test()" => "meson test"

> +
> +    Ex: ``$ mesin test --suite ‘DPDK:fast-tests’``

mesin => meson

<snip>

[dpdk-dev] [PATCH v4] doc: add meson ut info in prog guide

[Hari Kumar Vemula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6065 bytes --]

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
v4: Typos corrected
v3: Modified
v2: Removed enhancement details
---
 doc/guides/prog_guide/index.rst    |   1 +
 doc/guides/prog_guide/meson_ut.rst | 159 +++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 6726b1e8d..f4274573f 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -58,6 +58,7 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..9485b1e63
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,159 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+
+    Copyright(c) 2014-2018 Intel Corporation.
+
+.. _Meson:
+
+Meson_UT
+========
+
+The meson build for unit tests under different categories is supported using 'test/test/meson.build'.
+
+This document describes the below list in detail.
+
+*  Building and Running the unit tests.
+*  Grouping of testcases.
+*  Parallel and non parallel tests.
+*  Test suites.
+*  How to run different test suites.
+*  Support for skipped tests.
+
+
+
+Building and Running the unit tests
+-----------------------------------
+
+*  Create the meson build output folder using command.
+
+   ``$ meson <build_dir>``
+
+*  Enter into build output folder, which was created by above command.
+
+   ``$ cd build``
+
+*  Compile DPDK using `$ ninja`.
+   The output file of the build will be available in meson build folder.
+   After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
+
+*  Run the unit testcases.
+
+   ``$ ninja test`` (or) ``$ meson test``
+
+*   To run specific test case via meson command.
+
+   ``$ meson test <test case>`` (or) ``$ ninja test <test case>``
+
+
+
+Grouping of testcases
+---------------------
+
+Testcases has been grouped into below four different groups based on conditions
+of time duration and performance of the individual testcase.
+
+*  fast_parallel_test_names
+*  fast_non_parallel_test_names
+*  perf_test_names
+*  driver_test_names
+*  dump_test_names
+
+Parallel and non parallel tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+By default, meson will run test cases in parallel. However `is_parallel` argument
+of `test()` in meson.build can be used to run tests in parallel or non-parallel
+mode based on its functionality. Test cases marked with `is_parallel : true` will
+run in parallel and tests marked with `is_parallel : false` will run in non-parallel.
+While non-parallel test is running, no other test should run.
+Parallel and non parallel test cases are grouped under the `fast_parallel_test_names`
+and `fast_non_parallel_test_names`.
+
+
+
+Test suites
+~~~~~~~~~~~
+
+Tests are grouped into test suites in meson.build and these suite names can be provided
+as argument to `meson test` as `--suite ‘project_name:label’`.
+
+    Ex: ``$ meson test --suite ‘DPDK:fast-tests’``
+
+Running different test suites
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Below are commands to run testcases using option `suite`
+
+*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`
+   are run under 10seconds and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:fast-tests``
+
+*  Test cases from the group `perf_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:perf-tests``
+
+*  Test cases from the group `driver_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:driver-tests``
+
+*  Test cases from the group `dump_test_names` are run under 600 seconds
+   and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:dump-tests``
+
+
+
+Skipped testcases
+-----------------
+
+Some unit test cases have dependency on external libraries, driver modules or
+config flags, without which the test cases cannot be run. Such test cases
+should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
+test cases run user should enable relevant dependencies. Below are the few
+current scenarios when test cases are skipped:
+
+#. External library dependency paths are not set.
+#. Config flag for the dependent library is not enabled.
+#. Dependent driver modules are not installed on the system.
+
+Dependent library paths can be set using below
+
+*  Single path ``$ export LIBRARY_PATH=path``
+
+*  Multiple paths ``$ export LIBRARY_PATH=path1:path2:path3``
+
+Dependent library headers path can be stated as part of meson build command as below.
+
+*  Single path ``$ CFLAGS=-I/path meson build``
+
+*  Multiple paths ``$ CFLAGS=-I/path1 -I/path2 meson build``
+
+Below examples shows how to export libraries and their header paths.
+
+To specify single library at a time.
+
+        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/``
+        ``$ CFLAGS=-I/root/wireless_libs/zuc/include meson build``
+
+To specify multiple libraries at a time.
+
+        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
+                                                    ``libsso_kasumi/build/``
+        ``$ CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
+                                        ``libsso_kasumi/include meson build``
+
+
+
+Summery of Commands to run meson UTs
+------------------------------------
+
+*   To run all test cases
+       ``$ meson test``
+*   To run specific test
+       ``$ meson test testcase_name``
+        Ex:``$ meson test acl_autotest``
+*   To run specific test suite
+       ``$ meson test --suite DPDK:suite_name``
+        Ex:``$ meson test --suite DPDK:fast-tests``
-- 
2.17.2

Re: [dpdk-dev] [PATCH v4] doc: add meson ut info in prog guide

[Bruce Richardson]

On Fri, Jan 25, 2019 at 06:20:51AM +0000, Hari Kumar Vemula wrote:
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
> v4: Typos corrected
> v3: Modified
> v2: Removed enhancement details
> ---

Some sections I think need rewording to be written more from an end-user
rather than developer point of view, since this text is focused on running unit
tests rather than on writing them. Some additional spelling/grammar
corrections below also.

NOTE: Since this is in the programmers guide, maybe the intention is to
have it more at the programmer level. In which case, you need more info on
writing tests, and what return values should be used etc. other than
"TEST_SKIPPED", and can skip some of the detail on running tests.

Regards,
/Bruce

>  doc/guides/prog_guide/index.rst    |   1 +
>  doc/guides/prog_guide/meson_ut.rst | 159 +++++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+)
>  create mode 100644 doc/guides/prog_guide/meson_ut.rst
> 
> diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
> index 6726b1e8d..f4274573f 100644
> --- a/doc/guides/prog_guide/index.rst
> +++ b/doc/guides/prog_guide/index.rst
> @@ -58,6 +58,7 @@ Programmer's Guide
>      source_org
>      dev_kit_build_system
>      dev_kit_root_make_help
> +    meson_ut
>      extend_dpdk
>      build_app
>      ext_app_lib_make_help
> diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
> new file mode 100644
> index 000000000..9485b1e63
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,159 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +
> +    Copyright(c) 2014-2018 Intel Corporation.

Should be just 2019, or 2018-2019, since this doc was not started in 2014.

> +
> +.. _Meson:
> +
> +Meson_UT
> +========
> +
> +The meson build for unit tests under different categories is supported using 'test/test/meson.build'.
> +
This line is not wrong, just probably unnecessary. Consider omitting it, I
think, since this doc is about using the unit tests rather than about the
source code layout or individual files.

> +This document describes the below list in detail.
> +
> +*  Building and Running the unit tests.
> +*  Grouping of testcases.
> +*  Parallel and non parallel tests.
> +*  Test suites.
Are test suites not the same as the grouping of test cases?

> +*  How to run different test suites.
> +*  Support for skipped tests.
> +
> +
> +
> +Building and Running the unit tests
> +-----------------------------------
> +
> +*  Create the meson build output folder using command.
> +
> +   ``$ meson <build_dir>``
> +
> +*  Enter into build output folder, which was created by above command.
> +
> +   ``$ cd build``
> +
> +*  Compile DPDK using `$ ninja`.

Like the other commands above, this should be put on its own line.

> +   The output file of the build will be available in meson build folder.
> +   After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
> +
> +*  Run the unit testcases.
> +
> +   ``$ ninja test`` (or) ``$ meson test``
> +
> +*   To run specific test case via meson command.
> +
> +   ``$ meson test <test case>`` (or) ``$ ninja test <test case>``
> +
> +
> +
> +Grouping of testcases
> +---------------------
> +
> +Testcases has been grouped into below four different groups based on conditions

has -> have.
Remove the word "below" as it's unnecessary.

> +of time duration and performance of the individual testcase.
> +
> +*  fast_parallel_test_names
> +*  fast_non_parallel_test_names
> +*  perf_test_names
> +*  driver_test_names
> +*  dump_test_names

Since you are listing the groups, I think you should just put the groups
without using the official variable names, which looks wrong and doesn't
add any info. Instead the list should be:

* fast tests which can be run in parallel
* fast tests which must run serially
* performance tests
* driver tests
* tests which produce lists of objects as output, and therefore that need manual checking

> +
> +Parallel and non parallel tests
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +By default, meson will run test cases in parallel. However `is_parallel` argument
> +of `test()` in meson.build can be used to run tests in parallel or non-parallel
> +mode based on its functionality. Test cases marked with `is_parallel : true` will
> +run in parallel and tests marked with `is_parallel : false` will run in non-parallel.
> +While non-parallel test is running, no other test should run.
> +Parallel and non parallel test cases are grouped under the `fast_parallel_test_names`
> +and `fast_non_parallel_test_names`.
> +

The above paragraph is too low level. It can probably be dropped or
abbreviated to a single sentence included in the previous section. Again,
this text seems targeted at users, not developers, so variable names in the
meson.build file are irrelevant.

> +
> +
> +Test suites
> +~~~~~~~~~~~
> +
> +Tests are grouped into test suites in meson.build and these suite names can be provided
> +as argument to `meson test` as `--suite ???project_name:label???`.
> +
> +    Ex: ``$ meson test --suite ???DPDK:fast-tests???``
> +
> +Running different test suites
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Below are commands to run testcases using option `suite`
> +
> +*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`

Again, drop the variable names. Not relevant for users.

> +   are run under 10seconds and below is the meson command to run them.

"are run under 10seconds" -> "should take less than 10 seconds"

> +
> +   ``$ meson test --suite DPDK:fast-tests``
> +
> +*  Test cases from the group `perf_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$ meson test --suite DPDK:perf-tests``
> +
> +*  Test cases from the group `driver_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$ meson test --suite DPDK:driver-tests``
> +
> +*  Test cases from the group `dump_test_names` are run under 600 seconds

Since these just output things, do they really need a 600 second timeout?
Do we even need to mention it?

> +   and below is the meson command to run them.
> +
> +   ``$ meson test --suite DPDK:dump-tests``
> +
> +
> +
> +Skipped testcases
> +-----------------
> +
> +Some unit test cases have dependency on external libraries, driver modules or

"have dependency" -> "have a dependency"

> +config flags, without which the test cases cannot be run. Such test cases
> +should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
> +test cases run user should enable relevant dependencies. Below are the few
> +current scenarios when test cases are skipped:

Needs rewording for a more user-focused point of view, since TEST_SKIPPED
is an internal "magic number" in the code. Something like:
"Such test cases will be reported out as skipped if they cannot run. To
enable those test cases, the user should ensure the required dependencies
are met. Below are a few possible causes why tests may be skipped and how
they may be resolved:"

> +
> +#. External library dependency paths are not set.

The high-level problem is more that the library is not found, not that the
path is unset. The solution is to set the path to the dependency if
necessary.

> +#. Config flag for the dependent library is not enabled.
> +#. Dependent driver modules are not installed on the system.
> +
> +Dependent library paths can be set using below

"To help find missing libraries, the user can specify addition search paths
for those libraries as below:"

> +
> +*  Single path ``$ export LIBRARY_PATH=path``
> +
> +*  Multiple paths ``$ export LIBRARY_PATH=path1:path2:path3``
> +
> +Dependent library headers path can be stated as part of meson build command as below.

"Some functionality may be disabled due to library headers being missed as part of
the build. To specify an addition search path for headers at configuration
time, use one of the commands below:"

> +
> +*  Single path ``$ CFLAGS=-I/path meson build``
> +
> +*  Multiple paths ``$ CFLAGS=-I/path1 -I/path2 meson build``
> +
> +Below examples shows how to export libraries and their header paths.
> +
> +To specify single library at a time.
> +
> +        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/``
> +        ``$ CFLAGS=-I/root/wireless_libs/zuc/include meson build``
> +
> +To specify multiple libraries at a time.
> +
> +        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
> +                                                    ``libsso_kasumi/build/``
> +        ``$ CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
> +                                        ``libsso_kasumi/include meson build``
> +
> +
> +
> +Summery of Commands to run meson UTs

"Summary"

> +------------------------------------
> +
> +*   To run all test cases
> +       ``$ meson test``
> +*   To run specific test
> +       ``$ meson test testcase_name``
> +        Ex:``$ meson test acl_autotest``
> +*   To run specific test suite
> +       ``$ meson test --suite DPDK:suite_name``
> +        Ex:``$ meson test --suite DPDK:fast-tests``
> -- 
> 2.17.2
> 

[dpdk-dev] [PATCH v5] doc: add meson ut info in prog guide

[Hari Kumar Vemula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5371 bytes --]

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
v5: Modified
v4: Typos corrected
v3: Modified
v2: Removed enhancement details
---
 doc/guides/prog_guide/index.rst    |   1 +
 doc/guides/prog_guide/meson_ut.rst | 143 +++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 6726b1e8d..f4274573f 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -58,6 +58,7 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..e4b449049
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,143 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+
+    Copyright(c) 2018-2019 Intel Corporation.
+
+.. _Meson:
+
+Meson_UT
+========
+
+This document describes the below list in detail.
+
+*  Building and Running the unit tests.
+*  Grouping of testcases.
+*  How to run different test suites.
+*  Support for skipped tests.
+
+
+
+Building and Running the unit tests
+-----------------------------------
+
+*  Create the meson build output folder using command.
+
+   ``$ meson <build_dir>``
+
+*  Enter into build output folder, which was created by above command.
+
+   ``$ cd build``
+
+*  Compile DPDK using command.
+
+  ``$ ninja``
+
+  The output file of the build will be available in meson build folder.
+  After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
+
+*  Run the unit testcases.
+
+   ``$ ninja test`` (or) ``$ meson test``
+
+*   To run specific test case via meson command.
+
+   ``$ meson test <test case>`` (or) ``$ ninja test <test case>``
+
+
+
+Grouping of testcases
+---------------------
+
+Testcases have been grouped into four different groups based on conditions
+of time duration and performance of the individual testcase.
+
+*  fast tests which can be run in parallel
+*  fast tests which must run serially
+*  performance tests
+*  driver tests
+*  tests which produce lists of objects as output, and therefore that need manual checking
+
+Testcases can be run in parallel or non-parallel mode using the `is_parallel` argument
+of `test()` in meson.build
+
+These tests can be run using the argument to `meson test` as
+`--suite ‘project_name:label’`.
+
+    Ex: ``$ meson test --suite ‘DPDK:fast-tests’``
+
+Running different test suites
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Commands to run testcases using option `--suite`
+
+*  Fast Tests should take less than 10 seconds and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:fast-tests``
+
+*  Performance Tests should take less than 600 seconds and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:perf-tests``
+
+*  Driver Tests should take less than 600 seconds and below is the meson command to run them.
+
+   ``$ meson test --suite DPDK:driver-tests``
+
+*  Below is the meson command to run Dump Tests
+
+   ``$ meson test --suite DPDK:dump-tests``
+
+
+
+Skipped testcases
+-----------------
+
+Some unit test cases have a dependency on external libraries, driver modules or
+config flags, without which the test cases cannot be run. Such test cases will be reported
+out as skipped if they cannot run. To enable those test cases,
+the user should ensure the required dependencies are met.
+Below are a few possible causes why tests may be skipped and how they may be resolved:
+
+#. External library is not found.
+#. Config flag for the dependent library is not enabled.
+#. Dependent driver modules are not installed on the system.
+
+To help find missing libraries, the user can specify addition search paths
+for those libraries as below:
+
+*  Single path ``$ export LIBRARY_PATH=path``
+
+*  Multiple paths ``$ export LIBRARY_PATH=path1:path2:path3``
+
+Some functionality may be disabled due to library headers being missed as part of the build.
+To specify an addition search path for headers at configuration time, use one of the commands below:
+
+*  Single path ``$ CFLAGS=-I/path meson build``
+
+*  Multiple paths ``$ CFLAGS=-I/path1 -I/path2 meson build``
+
+Below examples shows how to export libraries and their header paths.
+
+To specify single library at a time.
+
+        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/``
+        ``$ CFLAGS=-I/root/wireless_libs/zuc/include meson build``
+
+To specify multiple libraries at a time.
+
+        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
+                                                    ``libsso_kasumi/build/``
+        ``$ CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
+                                        ``libsso_kasumi/include meson build``
+
+
+
+Summary
+--------
+
+*   To run all test cases
+       ``$ meson test``
+*   To run specific test
+       ``$ meson test testcase_name``
+        Ex:``$ meson test acl_autotest``
+*   To run specific test suite
+       ``$ meson test --suite DPDK:suite_name``
+        Ex:``$ meson test --suite DPDK:fast-tests``
-- 
2.17.2

Re: [dpdk-dev] [PATCH v5] doc: add meson ut info in prog guide

[Bruce Richardson]

On Sat, Feb 02, 2019 at 10:28:26AM +0000, Hari Kumar Vemula wrote:
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v5] doc: add meson ut info in prog guide

[Thomas Monjalon]

02/02/2019 11:28, Hari Kumar Vemula:
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,143 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +
> +    Copyright(c) 2018-2019 Intel Corporation.

Why inserting a blank line between SPDX and Copyright.

> +
> +.. _Meson:
> +
> +Meson_UT
> +========

This is a title. Why having an underscore instead of space?
Why not saying Unit Tests? Or better, "Run Unit Tests with Meson".

> +
> +This document describes the below list in detail.
> +
> +*  Building and Running the unit tests.
> +*  Grouping of testcases.
> +*  How to run different test suites.
> +*  Support for skipped tests.

This list is useless because automatically generated by Sphinx.

Sorry, I stop reading here.
Marko, John, did you try to review this patch?

Re: [dpdk-dev] [PATCH v5] doc: add meson ut info in prog guide

[Thomas Monjalon]

02/02/2019 11:28, Hari Kumar Vemula:
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,143 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +
> +    Copyright(c) 2018-2019 Intel Corporation.

Why inserting a blank line between SPDX and Copyright.

> +
> +.. _Meson:
> +
> +Meson_UT
> +========

This is a title. Why having an underscore instead of space?
Why not saying Unit Tests? Or better, "Run Unit Tests with Meson".

> +
> +This document describes the below list in detail.
> +
> +*  Building and Running the unit tests.
> +*  Grouping of testcases.
> +*  How to run different test suites.
> +*  Support for skipped tests.

This list is useless because automatically generated by Sphinx.

Sorry, I stop reading here.
Marko, John, did you try to review this patch?

Re: [dpdk-dev] [PATCH v5] doc: add meson ut info in prog guide

[Mcnamara, John]

> > Sorry, I stop reading here.
> Marko, John, did you try to review this patch?

Yes. I will review offline.

John

Re: [dpdk-dev] [PATCH v5] doc: add meson ut info in prog guide

[Mcnamara, John]

> > Sorry, I stop reading here.
> Marko, John, did you try to review this patch?

Yes. I will review offline.

John

[dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Hari Kumar Vemula]

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
v6: Updated comments
v5: Modified
v4: Typos corrected
v3: Modified
v2: Removed enhancement details
---
 doc/guides/prog_guide/index.rst    |   1 +
 doc/guides/prog_guide/meson_ut.rst | 151 +++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 692409af8..9465bc8e6 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -60,6 +60,7 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..e0aa15389
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,151 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+
+    Copyright(c) 2018-2019 Intel Corporation.
+
+.. _meson_unit_tests:
+
+Running DPDK Unit Tests with Meson
+==================================
+
+This section describes how to run testcases with the DPDK meson build system.
+
+
+Building and running the unit tests
+-----------------------------------
+
+* Create the meson build output folder using the following command::
+
+      $ meson <build_dir>
+
+* Enter into build output folder, which was created by above command::
+
+      $ cd build
+
+* Compile DPDK using command::
+
+      $ ninja
+
+The output file of the build will be available in meson build folder. After
+a successful ninja command, the binary ``dpdk-test`` is created in
+``build/test/test/``.
+
+* Run the unit testcases::
+
+      $ ninja test
+      # or
+      $ meson test
+
+* To run specific test case via meson::
+
+      $ meson test <test case>
+      # or
+      $ ninja test <test case>
+
+
+Grouping of testcases
+---------------------
+
+Testcases have been grouped into four different groups based on conditions
+of time duration and performance of the individual testcase.
+
+* Fast tests which can be run in parallel.
+* Fast tests which must run serially.
+* Performance tests.
+* Driver tests.
+* Tests which produce lists of objects as output, and therefore that need
+  manual checking.
+
+Testcases can be run in parallel or non-parallel mode using the ``is_parallel`` argument
+of ``test()`` in meson.build
+
+These tests can be run using the argument to ``meson test`` as
+``--suite project_name:label``.
+
+For example::
+
+    $ meson test --suite DPDK:fast-tests
+
+
+The project name is optional so the following is equivalent to the previous
+command::
+
+
+    $ meson test --suite fast-tests
+
+
+Running different test suites
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The following commands are some examples of how to run testcases using option
+``--suite``:
+
+* Fast Tests should take less than 10 seconds. The meson command to run them
+  is::
+
+      $ meson test --suite DPDK:fast-tests
+
+* Performance Tests should take less than 600 seconds. The meson command to
+  run them is::
+
+      $ meson test --suite DPDK:perf-tests
+
+* Driver Tests should take less than 600 seconds. The meson command to run
+  them is::
+
+      $ meson test --suite DPDK:driver-tests
+
+* The meson command to run Dump Tests is::
+
+      $ meson test --suite DPDK:dump-tests
+
+
+Dealing with skipped testcases
+------------------------------
+
+Some unit test cases have a dependency on external libraries, driver modules
+or config flags, without which the test cases cannot be run. Such test cases
+will be reported as skipped if they cannot run. To enable those test cases,
+the user should ensure the required dependencies are met.  Below are a few
+possible causes why tests may be skipped and how they may be resolved:
+
+#. Optional external libraries are not found.
+#. Config flags for the dependent library are not enabled.
+#. Dependent driver modules are not installed on the system.
+
+To help find missing libraries, the user can specify addition search paths
+for those libraries as below:
+
+* Single path::
+
+      $ export LIBRARY_PATH=path
+
+* Multiple paths::
+
+      $ export LIBRARY_PATH=path1:path2:path3
+
+Some functionality may be disabled due to library headers being missed as part
+of the build. To specify an additional search path for headers at
+configuration time, use one of the commands below:
+
+*  Single path::
+
+       $ CFLAGS=-I/path meson build
+
+*  Multiple paths::
+
+       $ CFLAGS=-I/path1 -I/path2 meson build
+
+Below are some examples that show how to export libraries and their header
+paths.
+
+To specify a single library at a time::
+
+    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
+    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
+
+To specify multiple libraries at a time::
+
+    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
+    $ CFLAGS=-I/path/zuc/include \
+             -I/path/libsso_kasumi/include \
+	     meson build
-- 
2.14.1

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Thomas Monjalon]

Hi please find some comments below:

06/06/2019 13:59, Hari Kumar Vemula:
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,151 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +

Useless blank line.

> +    Copyright(c) 2018-2019 Intel Corporation.
> +
> +.. _meson_unit_tests:

Useless anchor. The doc can be referenced with :doc: links.

> +
> +Running DPDK Unit Tests with Meson
> +==================================
> +
> +This section describes how to run testcases with the DPDK meson build system.

Here and below, "testcases" should be split in two words.

> +
> +
> +Building and running the unit tests
> +-----------------------------------
> +
> +* Create the meson build output folder using the following command::
> +
> +      $ meson <build_dir>
> +
> +* Enter into build output folder, which was created by above command::
> +
> +      $ cd build

Should be the same as above: <build_dir>

> +
> +* Compile DPDK using command::
> +
> +      $ ninja

Do we really need to repeat above basic steps?
Would be easier to just reference another guide about meson.
I think doc/build-sdk-meson.txt should be moved to .rst.

> +
> +The output file of the build will be available in meson build folder. After
> +a successful ninja command, the binary ``dpdk-test`` is created in
> +``build/test/test/``.

Again, "build" is an example directory.

> +
> +* Run the unit testcases::
> +
> +      $ ninja test
> +      # or
> +      $ meson test
> +
> +* To run specific test case via meson::
> +
> +      $ meson test <test case>
> +      # or
> +      $ ninja test <test case>

Would be worth to mention why meson or ninja can be used.

> +
> +
> +Grouping of testcases
> +---------------------
> +
> +Testcases have been grouped into four different groups based on conditions
> +of time duration and performance of the individual testcase.

Grouping has changed recently.
This part should be updated please.

> +
> +* Fast tests which can be run in parallel.
> +* Fast tests which must run serially.
> +* Performance tests.
> +* Driver tests.
> +* Tests which produce lists of objects as output, and therefore that need
> +  manual checking.
> +
> +Testcases can be run in parallel or non-parallel mode using the ``is_parallel`` argument
> +of ``test()`` in meson.build
> +
> +These tests can be run using the argument to ``meson test`` as
> +``--suite project_name:label``.
> +
> +For example::
> +
> +    $ meson test --suite DPDK:fast-tests
> +
> +
> +The project name is optional so the following is equivalent to the previous
> +command::
> +
> +
> +    $ meson test --suite fast-tests
> +
> +
> +Running different test suites
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The following commands are some examples of how to run testcases using option
> +``--suite``:
> +
> +* Fast Tests should take less than 10 seconds. The meson command to run them
> +  is::
> +
> +      $ meson test --suite DPDK:fast-tests
> +
> +* Performance Tests should take less than 600 seconds. The meson command to
> +  run them is::
> +
> +      $ meson test --suite DPDK:perf-tests
> +
> +* Driver Tests should take less than 600 seconds. The meson command to run
> +  them is::
> +
> +      $ meson test --suite DPDK:driver-tests
> +
> +* The meson command to run Dump Tests is::
> +
> +      $ meson test --suite DPDK:dump-tests

Would be simpler to just list the suites.

> +
> +
> +Dealing with skipped testcases
> +------------------------------
> +
> +Some unit test cases have a dependency on external libraries, driver modules
> +or config flags, without which the test cases cannot be run. Such test cases
> +will be reported as skipped if they cannot run. To enable those test cases,
> +the user should ensure the required dependencies are met.  Below are a few
> +possible causes why tests may be skipped and how they may be resolved:
> +
> +#. Optional external libraries are not found.
> +#. Config flags for the dependent library are not enabled.
> +#. Dependent driver modules are not installed on the system.
> +
> +To help find missing libraries, the user can specify addition search paths

addition -> additional ?

> +for those libraries as below:
> +
> +* Single path::
> +
> +      $ export LIBRARY_PATH=path
> +
> +* Multiple paths::
> +
> +      $ export LIBRARY_PATH=path1:path2:path3
> +
> +Some functionality may be disabled due to library headers being missed as part
> +of the build. To specify an additional search path for headers at
> +configuration time, use one of the commands below:
> +
> +*  Single path::
> +
> +       $ CFLAGS=-I/path meson build
> +
> +*  Multiple paths::
> +
> +       $ CFLAGS=-I/path1 -I/path2 meson build

Some quotes are missing to set multiple paths.

> +
> +Below are some examples that show how to export libraries and their header
> +paths.
> +
> +To specify a single library at a time::
> +
> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
> +
> +To specify multiple libraries at a time::
> +
> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
> +    $ CFLAGS=-I/path/zuc/include \
> +             -I/path/libsso_kasumi/include \
> +	     meson build

Why export is used for LIBRARY_PATH and not CFLAGS?
I think both variables can be exported or prepend the meson command?

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Aaron Conole]

Thomas Monjalon <thomas@monjalon.net> writes:

> Hi please find some comments below:
>
> 06/06/2019 13:59, Hari Kumar Vemula:
>> +++ b/doc/guides/prog_guide/meson_ut.rst
>> @@ -0,0 +1,151 @@
>> +..  SPDX-License-Identifier: BSD-3-Clause
>> +
>
> Useless blank line.
>
>> +    Copyright(c) 2018-2019 Intel Corporation.
>> +
>> +.. _meson_unit_tests:
>
> Useless anchor. The doc can be referenced with :doc: links.
>
>> +
>> +Running DPDK Unit Tests with Meson
>> +==================================
>> +
>> +This section describes how to run testcases with the DPDK meson build system.
>
> Here and below, "testcases" should be split in two words.
>
>> +
>> +
>> +Building and running the unit tests
>> +-----------------------------------
>> +
>> +* Create the meson build output folder using the following command::
>> +
>> +      $ meson <build_dir>
>> +
>> +* Enter into build output folder, which was created by above command::
>> +
>> +      $ cd build
>
> Should be the same as above: <build_dir>
>
>> +
>> +* Compile DPDK using command::
>> +
>> +      $ ninja
>
> Do we really need to repeat above basic steps?
> Would be easier to just reference another guide about meson.
> I think doc/build-sdk-meson.txt should be moved to .rst.

+1

>> +
>> +The output file of the build will be available in meson build folder. After
>> +a successful ninja command, the binary ``dpdk-test`` is created in
>> +``build/test/test/``.
>
> Again, "build" is an example directory.
>
>> +
>> +* Run the unit testcases::
>> +
>> +      $ ninja test
>> +      # or
>> +      $ meson test
>> +
>> +* To run specific test case via meson::
>> +
>> +      $ meson test <test case>
>> +      # or
>> +      $ ninja test <test case>
>
> Would be worth to mention why meson or ninja can be used.
>
>> +
>> +
>> +Grouping of testcases
>> +---------------------
>> +
>> +Testcases have been grouped into four different groups based on conditions
>> +of time duration and performance of the individual testcase.
>
> Grouping has changed recently.
> This part should be updated please.
>
>> +
>> +* Fast tests which can be run in parallel.
>> +* Fast tests which must run serially.
>> +* Performance tests.
>> +* Driver tests.
>> +* Tests which produce lists of objects as output, and therefore that need
>> +  manual checking.
>> +
>> +Testcases can be run in parallel or non-parallel mode using the ``is_parallel`` argument
>> +of ``test()`` in meson.build
>> +
>> +These tests can be run using the argument to ``meson test`` as
>> +``--suite project_name:label``.
>> +
>> +For example::
>> +
>> +    $ meson test --suite DPDK:fast-tests
>> +
>> +
>> +The project name is optional so the following is equivalent to the previous
>> +command::
>> +
>> +
>> +    $ meson test --suite fast-tests
>> +
>> +
>> +Running different test suites
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +The following commands are some examples of how to run testcases using option
>> +``--suite``:

The following section is a bit misleading.  The limitation on run time
is per-test.  So 600 seconds in perf-tests is 600 seconds PER TEST.  IE:
if there are 10 tests, you'll be waiting up to 50 minutes.

>> +* Fast Tests should take less than 10 seconds. The meson command to run them
>> +  is::
>> +
>> +      $ meson test --suite DPDK:fast-tests
>> +
>> +* Performance Tests should take less than 600 seconds. The meson command to
>> +  run them is::
>> +
>> +      $ meson test --suite DPDK:perf-tests
>> +
>> +* Driver Tests should take less than 600 seconds. The meson command to run
>> +  them is::
>> +
>> +      $ meson test --suite DPDK:driver-tests
>> +
>> +* The meson command to run Dump Tests is::
>> +
>> +      $ meson test --suite DPDK:dump-tests
>
> Would be simpler to just list the suites.

Even better would be to provide a 1-liner that would dump the suites so
that new test suites wouldn't need to update the documentation.

>> +
>> +
>> +Dealing with skipped testcases
>> +------------------------------
>> +
>> +Some unit test cases have a dependency on external libraries, driver modules
>> +or config flags, without which the test cases cannot be run. Such test cases
>> +will be reported as skipped if they cannot run. To enable those test cases,
>> +the user should ensure the required dependencies are met.  Below are a few
>> +possible causes why tests may be skipped and how they may be resolved:
>> +
>> +#. Optional external libraries are not found.
>> +#. Config flags for the dependent library are not enabled.
>> +#. Dependent driver modules are not installed on the system.
>> +
>> +To help find missing libraries, the user can specify addition search paths
>
> addition -> additional ?
>
>> +for those libraries as below:
>> +
>> +* Single path::
>> +
>> +      $ export LIBRARY_PATH=path
>> +
>> +* Multiple paths::
>> +
>> +      $ export LIBRARY_PATH=path1:path2:path3
>> +
>> +Some functionality may be disabled due to library headers being missed as part
>> +of the build. To specify an additional search path for headers at
>> +configuration time, use one of the commands below:
>> +
>> +*  Single path::
>> +
>> +       $ CFLAGS=-I/path meson build
>> +
>> +*  Multiple paths::
>> +
>> +       $ CFLAGS=-I/path1 -I/path2 meson build
>
> Some quotes are missing to set multiple paths.
>
>> +
>> +Below are some examples that show how to export libraries and their header
>> +paths.
>> +
>> +To specify a single library at a time::
>> +
>> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
>> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
>> +
>> +To specify multiple libraries at a time::
>> +
>> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
>> +    $ CFLAGS=-I/path/zuc/include \
>> +             -I/path/libsso_kasumi/include \
>> +	     meson build
>
> Why export is used for LIBRARY_PATH and not CFLAGS?
> I think both variables can be exported or prepend the meson command?

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Michael Santana Francisco]

On Mon, Jul 8, 2019 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
>
> Thomas Monjalon <thomas@monjalon.net> writes:
>
> > Hi please find some comments below:
> >
> > 06/06/2019 13:59, Hari Kumar Vemula:
> >> +++ b/doc/guides/prog_guide/meson_ut.rst
> >> @@ -0,0 +1,151 @@
> >> +..  SPDX-License-Identifier: BSD-3-Clause
> >> +
> >
> > Useless blank line.
> >
> >> +    Copyright(c) 2018-2019 Intel Corporation.
> >> +
> >> +.. _meson_unit_tests:
> >
> > Useless anchor. The doc can be referenced with :doc: links.
> >
> >> +
> >> +Running DPDK Unit Tests with Meson
> >> +==================================
> >> +
> >> +This section describes how to run testcases with the DPDK meson build system.
> >
> > Here and below, "testcases" should be split in two words.
> >
> >> +
> >> +
> >> +Building and running the unit tests
> >> +-----------------------------------
> >> +
> >> +* Create the meson build output folder using the following command::
> >> +
> >> +      $ meson <build_dir>
> >> +
> >> +* Enter into build output folder, which was created by above command::
> >> +
> >> +      $ cd build
> >
> > Should be the same as above: <build_dir>
> >
> >> +
> >> +* Compile DPDK using command::
> >> +
> >> +      $ ninja
> >
> > Do we really need to repeat above basic steps?
> > Would be easier to just reference another guide about meson.
> > I think doc/build-sdk-meson.txt should be moved to .rst.
>
> +1
>
> >> +
> >> +The output file of the build will be available in meson build folder. After
> >> +a successful ninja command, the binary ``dpdk-test`` is created in
> >> +``build/test/test/``.
> >
> > Again, "build" is an example directory.
> >
> >> +
> >> +* Run the unit testcases::
> >> +
> >> +      $ ninja test
> >> +      # or
> >> +      $ meson test
> >> +
> >> +* To run specific test case via meson::
> >> +
> >> +      $ meson test <test case>
> >> +      # or
> >> +      $ ninja test <test case>
> >
> > Would be worth to mention why meson or ninja can be used.
> >
> >> +
> >> +
> >> +Grouping of testcases
> >> +---------------------
> >> +
> >> +Testcases have been grouped into four different groups based on conditions
> >> +of time duration and performance of the individual testcase.
> >
> > Grouping has changed recently.
> > This part should be updated please.
> >
> >> +
> >> +* Fast tests which can be run in parallel.
> >> +* Fast tests which must run serially.
> >> +* Performance tests.
> >> +* Driver tests.
> >> +* Tests which produce lists of objects as output, and therefore that need
> >> +  manual checking.
> >> +
> >> +Testcases can be run in parallel or non-parallel mode using the ``is_parallel`` argument
> >> +of ``test()`` in meson.build
> >> +
> >> +These tests can be run using the argument to ``meson test`` as
> >> +``--suite project_name:label``.
> >> +
> >> +For example::
> >> +
> >> +    $ meson test --suite DPDK:fast-tests
> >> +
> >> +
> >> +The project name is optional so the following is equivalent to the previous
> >> +command::
> >> +
> >> +
> >> +    $ meson test --suite fast-tests
> >> +
> >> +
> >> +Running different test suites
> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> +
> >> +The following commands are some examples of how to run testcases using option
> >> +``--suite``:
>
> The following section is a bit misleading.  The limitation on run time
> is per-test.  So 600 seconds in perf-tests is 600 seconds PER TEST.  IE:
> if there are 10 tests, you'll be waiting up to 50 minutes.
>
> >> +* Fast Tests should take less than 10 seconds. The meson command to run them
> >> +  is::
> >> +
> >> +      $ meson test --suite DPDK:fast-tests
> >> +
> >> +* Performance Tests should take less than 600 seconds. The meson command to
> >> +  run them is::
> >> +
> >> +      $ meson test --suite DPDK:perf-tests
> >> +
> >> +* Driver Tests should take less than 600 seconds. The meson command to run
> >> +  them is::
> >> +
> >> +      $ meson test --suite DPDK:driver-tests
> >> +
> >> +* The meson command to run Dump Tests is::
> >> +
> >> +      $ meson test --suite DPDK:dump-tests
> >
> > Would be simpler to just list the suites.
>
> Even better would be to provide a 1-liner that would dump the suites so
> that new test suites wouldn't need to update the documentation.
Worth mentioning that you can run `meson test --list` to see a list of
all available tests
>
> >> +
> >> +
> >> +Dealing with skipped testcases
> >> +------------------------------
> >> +
> >> +Some unit test cases have a dependency on external libraries, driver modules
> >> +or config flags, without which the test cases cannot be run. Such test cases
> >> +will be reported as skipped if they cannot run. To enable those test cases,
> >> +the user should ensure the required dependencies are met.  Below are a few
> >> +possible causes why tests may be skipped and how they may be resolved:
> >> +
> >> +#. Optional external libraries are not found.
> >> +#. Config flags for the dependent library are not enabled.
> >> +#. Dependent driver modules are not installed on the system.
> >> +
> >> +To help find missing libraries, the user can specify addition search paths
> >
> > addition -> additional ?
> >
> >> +for those libraries as below:
> >> +
> >> +* Single path::
> >> +
> >> +      $ export LIBRARY_PATH=path
> >> +
> >> +* Multiple paths::
> >> +
> >> +      $ export LIBRARY_PATH=path1:path2:path3
> >> +
> >> +Some functionality may be disabled due to library headers being missed as part
> >> +of the build. To specify an additional search path for headers at
> >> +configuration time, use one of the commands below:
> >> +
> >> +*  Single path::
> >> +
> >> +       $ CFLAGS=-I/path meson build
> >> +
> >> +*  Multiple paths::
> >> +
> >> +       $ CFLAGS=-I/path1 -I/path2 meson build
> >
> > Some quotes are missing to set multiple paths.
> >
> >> +
> >> +Below are some examples that show how to export libraries and their header
> >> +paths.
> >> +
> >> +To specify a single library at a time::
> >> +
> >> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
> >> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
> >> +
> >> +To specify multiple libraries at a time::
> >> +
> >> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
> >> +    $ CFLAGS=-I/path/zuc/include \
> >> +             -I/path/libsso_kasumi/include \
> >> +         meson build
> >
> > Why export is used for LIBRARY_PATH and not CFLAGS?
> > I think both variables can be exported or prepend the meson command?

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Parthasarathy, JananeeX M]

Hi,

>-----Original Message-----
>From: Michael Santana Francisco [mailto:msantana@redhat.com]
>Sent: Wednesday, July 10, 2019 12:28 AM
>To: Aaron Conole <aconole@redhat.com>
>Cc: Thomas Monjalon <thomas@monjalon.net>; Hari Kumar Vemula
><hari.kumarx.vemula@intel.com>; dev <dev@dpdk.org>; Pattan, Reshma
><reshma.pattan@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
>Kovacevic, Marko <marko.kovacevic@intel.com>; Parthasarathy, JananeeX M
><jananeex.m.parthasarathy@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; David Marchand
><david.marchand@redhat.com>
>Subject: Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide
>
>On Mon, Jul 8, 2019 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
>>
>> Thomas Monjalon <thomas@monjalon.net> writes:
>>
>> > Hi please find some comments below:
>> >
>> > 06/06/2019 13:59, Hari Kumar Vemula:
>> >> +++ b/doc/guides/prog_guide/meson_ut.rst
>> >> @@ -0,0 +1,151 @@
>> >> +..  SPDX-License-Identifier: BSD-3-Clause
>> >> +
>> >
>> > Useless blank line.
>> >
>> >> +    Copyright(c) 2018-2019 Intel Corporation.
>> >> +
>> >> +.. _meson_unit_tests:
>> >
>> > Useless anchor. The doc can be referenced with :doc: links.
>> >
>> >> +
>> >> +Running DPDK Unit Tests with Meson
>> >> +==================================
>> >> +
>> >> +This section describes how to run testcases with the DPDK meson build
>system.
>> >
>> > Here and below, "testcases" should be split in two words.

Will correct it.

>> >
>> >> +
>> >> +
>> >> +Building and running the unit tests
>> >> +-----------------------------------
>> >> +
>> >> +* Create the meson build output folder using the following command::
>> >> +
>> >> +      $ meson <build_dir>
>> >> +
>> >> +* Enter into build output folder, which was created by above command::
>> >> +
>> >> +      $ cd build
>> >
>> > Should be the same as above: <build_dir>

Will change accordingly.

>> >
>> >> +
>> >> +* Compile DPDK using command::
>> >> +
>> >> +      $ ninja
>> >
>> > Do we really need to repeat above basic steps?
>> > Would be easier to just reference another guide about meson.
>> > I think doc/build-sdk-meson.txt should be moved to .rst.
>>
>> +1
>>

This doc helps to run UT, having basic steps in same page will help user to go through together and execute the same.
Just for few lines moving back and forth to different pages might be bit confusing.
Anyway still if you would prefer to remove these then only 2 sections will be available in this doc.
Please let us know if it is ok.

>> >> +
>> >> +The output file of the build will be available in meson build
>> >> +folder. After a successful ninja command, the binary ``dpdk-test``
>> >> +is created in ``build/test/test/``.
>> >
>> > Again, "build" is an example directory.
>> >
>> >> +
>> >> +* Run the unit testcases::
>> >> +
>> >> +      $ ninja test
>> >> +      # or
>> >> +      $ meson test
>> >> +
>> >> +* To run specific test case via meson::
>> >> +
>> >> +      $ meson test <test case>
>> >> +      # or
>> >> +      $ ninja test <test case>
>> >
>> > Would be worth to mention why meson or ninja can be used.
>> >

OK
>> >> +
>> >> +
>> >> +Grouping of testcases
>> >> +---------------------
>> >> +
>> >> +Testcases have been grouped into four different groups based on
>> >> +conditions of time duration and performance of the individual testcase.
>> >
>> > Grouping has changed recently.
>> > This part should be updated please.
	
Thanks for the info. We observed that fast parallel test group is removed. 
Now we have four groups - fast, perf, driver, debug - all in non-parallel mode We will update the same.

>> >
>> >> +
>> >> +* Fast tests which can be run in parallel.
>> >> +* Fast tests which must run serially.
>> >> +* Performance tests.
>> >> +* Driver tests.
>> >> +* Tests which produce lists of objects as output, and therefore
>> >> +that need
>> >> +  manual checking.
>> >> +
>> >> +Testcases can be run in parallel or non-parallel mode using the
>> >> +``is_parallel`` argument of ``test()`` in meson.build
>> >> +
>> >> +These tests can be run using the argument to ``meson test`` as
>> >> +``--suite project_name:label``.
>> >> +
>> >> +For example::
>> >> +
>> >> +    $ meson test --suite DPDK:fast-tests
>> >> +
>> >> +
>> >> +The project name is optional so the following is equivalent to the
>> >> +previous
>> >> +command::
>> >> +
>> >> +
>> >> +    $ meson test --suite fast-tests
>> >> +
>> >> +
>> >> +Running different test suites
>> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >> +
>> >> +The following commands are some examples of how to run testcases
>> >> +using option
>> >> +``--suite``:
>>
>> The following section is a bit misleading.  The limitation on run time
>> is per-test.  So 600 seconds in perf-tests is 600 seconds PER TEST.  IE:
>> if there are 10 tests, you'll be waiting up to 50 minutes.
>>
>> >> +* Fast Tests should take less than 10 seconds. The meson command
>> >> +to run them
>> >> +  is::
>> >> +
>> >> +      $ meson test --suite DPDK:fast-tests
>> >> +
>> >> +* Performance Tests should take less than 600 seconds. The meson
>> >> +command to
>> >> +  run them is::
>> >> +
>> >> +      $ meson test --suite DPDK:perf-tests
>> >> +
>> >> +* Driver Tests should take less than 600 seconds. The meson
>> >> +command to run
>> >> +  them is::
>> >> +
>> >> +      $ meson test --suite DPDK:driver-tests
>> >> +
>> >> +* The meson command to run Dump Tests is::
>> >> +
>> >> +      $ meson test --suite DPDK:dump-tests
>> >
>> > Would be simpler to just list the suites.
>>
>> Even better would be to provide a 1-liner that would dump the suites
>> so that new test suites wouldn't need to update the documentation.
>Worth mentioning that you can run `meson test --list` to see a list of all
>available tests
>>

Will update the same and remove multiple lines.

>> >> +
>> >> +
>> >> +Dealing with skipped testcases
>> >> +------------------------------
>> >> +
>> >> +Some unit test cases have a dependency on external libraries,
>> >> +driver modules or config flags, without which the test cases
>> >> +cannot be run. Such test cases will be reported as skipped if they
>> >> +cannot run. To enable those test cases, the user should ensure the
>> >> +required dependencies are met.  Below are a few possible causes why
>tests may be skipped and how they may be resolved:
>> >> +
>> >> +#. Optional external libraries are not found.
>> >> +#. Config flags for the dependent library are not enabled.
>> >> +#. Dependent driver modules are not installed on the system.
>> >> +
>> >> +To help find missing libraries, the user can specify addition
>> >> +search paths
>> >
>> > addition -> additional ?
>> >
>> >> +for those libraries as below:
>> >> +
>> >> +* Single path::
>> >> +
>> >> +      $ export LIBRARY_PATH=path
>> >> +
>> >> +* Multiple paths::
>> >> +
>> >> +      $ export LIBRARY_PATH=path1:path2:path3
>> >> +
>> >> +Some functionality may be disabled due to library headers being
>> >> +missed as part of the build. To specify an additional search path
>> >> +for headers at configuration time, use one of the commands below:
>> >> +
>> >> +*  Single path::
>> >> +
>> >> +       $ CFLAGS=-I/path meson build
>> >> +
>> >> +*  Multiple paths::
>> >> +
>> >> +       $ CFLAGS=-I/path1 -I/path2 meson build
>> >
>> > Some quotes are missing to set multiple paths.

Is <build_dir>  meant here?

>> >
>> >> +
>> >> +Below are some examples that show how to export libraries and
>> >> +their header paths.
>> >> +
>> >> +To specify a single library at a time::
>> >> +
>> >> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
>> >> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
>> >> +
>> >> +To specify multiple libraries at a time::
>> >> +
>> >> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
>> >> +    $ CFLAGS=-I/path/zuc/include \
>> >> +             -I/path/libsso_kasumi/include \
>> >> +         meson build
>> >
>> > Why export is used for LIBRARY_PATH and not CFLAGS?
>> > I think both variables can be exported or prepend the meson command?

CFLAGS given in meson command works and  also CFLAGS can be exported.
LIBRARY_PATH  cannot be prepended to meson command. We tried but it is not reflecting the required values.
Environment variables set using export is considered and not as command line args of meson command.


Will send the next version patch addressing these comments.

Thanks
M.P.Jananee

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Thomas Monjalon]

22/07/2019 14:39, Parthasarathy, JananeeX M:
>From: Michael Santana Francisco [mailto:msantana@redhat.com]
> >On Mon, Jul 8, 2019 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
> >> Thomas Monjalon <thomas@monjalon.net> writes:
> >> >> +Building and running the unit tests
> >> >> +-----------------------------------
> >> >> +
> >> >> +* Create the meson build output folder using the following command::
> >> >> +
> >> >> +      $ meson <build_dir>
> >> >> +
> >> >> +* Enter into build output folder, which was created by above command::
> >> >> +
> >> >> +      $ cd build
> >> >
> >> > Should be the same as above: <build_dir>
> 
> Will change accordingly.
> 
> >> >
> >> >> +
> >> >> +* Compile DPDK using command::
> >> >> +
> >> >> +      $ ninja
> >> >
> >> > Do we really need to repeat above basic steps?
> >> > Would be easier to just reference another guide about meson.
> >> > I think doc/build-sdk-meson.txt should be moved to .rst.
> >>
> >> +1
> 
> This doc helps to run UT, having basic steps in same page will help user to go through together and execute the same.
> Just for few lines moving back and forth to different pages might be bit confusing.
> Anyway still if you would prefer to remove these then only 2 sections will be available in this doc.
> Please let us know if it is ok.

I think it is better to avoid repetition.


> >> >> +*  Multiple paths::
> >> >> +
> >> >> +       $ CFLAGS=-I/path1 -I/path2 meson build
> >> >
> >> > Some quotes are missing to set multiple paths.
> 
> Is <build_dir>  meant here?

I am just saying that space-separated value require some quotes.

> >> >> +Below are some examples that show how to export libraries and
> >> >> +their header paths.
> >> >> +
> >> >> +To specify a single library at a time::
> >> >> +
> >> >> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
> >> >> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
> >> >> +
> >> >> +To specify multiple libraries at a time::
> >> >> +
> >> >> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
> >> >> +    $ CFLAGS=-I/path/zuc/include \
> >> >> +             -I/path/libsso_kasumi/include \
> >> >> +         meson build
> >> >
> >> > Why export is used for LIBRARY_PATH and not CFLAGS?
> >> > I think both variables can be exported or prepend the meson command?
> 
> CFLAGS given in meson command works and  also CFLAGS can be exported.
> LIBRARY_PATH  cannot be prepended to meson command. We tried but it is not reflecting the required values.
> Environment variables set using export is considered and not as command line args of meson command.

Please we need to understand why LIBRARY_PATH is not working
when preprended in the meson command.
Do you have more informations?

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Bruce Richardson]

On Mon, Jul 22, 2019 at 02:53:34PM +0200, Thomas Monjalon wrote:
> 22/07/2019 14:39, Parthasarathy, JananeeX M:
> >From: Michael Santana Francisco [mailto:msantana@redhat.com]
> > >On Mon, Jul 8, 2019 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
> > >> Thomas Monjalon <thomas@monjalon.net> writes:
> > >> >> +Building and running the unit tests
> > >> >> +-----------------------------------
> > >> >> +
> > >> >> +* Create the meson build output folder using the following command::
> > >> >> +
> > >> >> +      $ meson <build_dir>
> > >> >> +
> > >> >> +* Enter into build output folder, which was created by above command::
> > >> >> +
> > >> >> +      $ cd build
> > >> >
> > >> > Should be the same as above: <build_dir>
> > 
> > Will change accordingly.
> > 
> > >> >
> > >> >> +
> > >> >> +* Compile DPDK using command::
> > >> >> +
> > >> >> +      $ ninja
> > >> >
> > >> > Do we really need to repeat above basic steps?
> > >> > Would be easier to just reference another guide about meson.
> > >> > I think doc/build-sdk-meson.txt should be moved to .rst.
> > >>
> > >> +1
> > 
> > This doc helps to run UT, having basic steps in same page will help user to go through together and execute the same.
> > Just for few lines moving back and forth to different pages might be bit confusing.
> > Anyway still if you would prefer to remove these then only 2 sections will be available in this doc.
> > Please let us know if it is ok.
> 
> I think it is better to avoid repetition.
> 
> 
> > >> >> +*  Multiple paths::
> > >> >> +
> > >> >> +       $ CFLAGS=-I/path1 -I/path2 meson build
> > >> >
> > >> > Some quotes are missing to set multiple paths.
> > 
> > Is <build_dir>  meant here?
> 
> I am just saying that space-separated value require some quotes.
> 
> > >> >> +Below are some examples that show how to export libraries and
> > >> >> +their header paths.
> > >> >> +
> > >> >> +To specify a single library at a time::
> > >> >> +
> > >> >> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
> > >> >> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
> > >> >> +
> > >> >> +To specify multiple libraries at a time::
> > >> >> +
> > >> >> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
> > >> >> +    $ CFLAGS=-I/path/zuc/include \
> > >> >> +             -I/path/libsso_kasumi/include \
> > >> >> +         meson build
> > >> >
> > >> > Why export is used for LIBRARY_PATH and not CFLAGS?
> > >> > I think both variables can be exported or prepend the meson command?
> > 
> > CFLAGS given in meson command works and  also CFLAGS can be exported.
> > LIBRARY_PATH  cannot be prepended to meson command. We tried but it is not reflecting the required values.
> > Environment variables set using export is considered and not as command line args of meson command.
> 
> Please we need to understand why LIBRARY_PATH is not working
> when preprended in the meson command.
> Do you have more informations?
>

Rather than using the environment, one can also use "-Dc_args='...'
-Dc_link_args='...'" when calling meson.

Otherwise, I agree with Thomas, we should find out why the LIBRARY_PATH
doesn't work when passed in directly. I'm not aware of any reason it
shouldn't work.

/Bruce 

Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide

[Parthasarathy, JananeeX M]

Hi,

>-----Original Message-----
>From: Bruce Richardson [mailto:bruce.richardson@intel.com]
>Sent: Monday, July 22, 2019 7:23 PM
>To: Thomas Monjalon <thomas@monjalon.net>
>Cc: Parthasarathy, JananeeX M <jananeex.m.parthasarathy@intel.com>;
>dev@dpdk.org; Michael Santana Francisco <msantana@redhat.com>; Aaron
>Conole <aconole@redhat.com>; Hari Kumar Vemula
><hari.kumarx.vemula@intel.com>; Pattan, Reshma
><reshma.pattan@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
>Kovacevic, Marko <marko.kovacevic@intel.com>; David Marchand
><david.marchand@redhat.com>
>Subject: Re: [dpdk-dev] [PATCH v6] doc: add meson ut info in prog guide
>
>On Mon, Jul 22, 2019 at 02:53:34PM +0200, Thomas Monjalon wrote:
>> 22/07/2019 14:39, Parthasarathy, JananeeX M:
>> >From: Michael Santana Francisco [mailto:msantana@redhat.com]
>> > >On Mon, Jul 8, 2019 at 4:18 PM Aaron Conole <aconole@redhat.com>
>wrote:
>> > >> Thomas Monjalon <thomas@monjalon.net> writes:
>> > >> >> +Building and running the unit tests
>> > >> >> +-----------------------------------
>> > >> >> +
>> > >> >> +* Create the meson build output folder using the following
>command::
>> > >> >> +
>> > >> >> +      $ meson <build_dir>
>> > >> >> +
>> > >> >> +* Enter into build output folder, which was created by above
>command::
>> > >> >> +
>> > >> >> +      $ cd build
>> > >> >
>> > >> > Should be the same as above: <build_dir>
>> >
>> > Will change accordingly.
>> >
>> > >> >
>> > >> >> +
>> > >> >> +* Compile DPDK using command::
>> > >> >> +
>> > >> >> +      $ ninja
>> > >> >
>> > >> > Do we really need to repeat above basic steps?
>> > >> > Would be easier to just reference another guide about meson.
>> > >> > I think doc/build-sdk-meson.txt should be moved to .rst.
>> > >>
>> > >> +1
>> >
>> > This doc helps to run UT, having basic steps in same page will help user to
>go through together and execute the same.
>> > Just for few lines moving back and forth to different pages might be bit
>confusing.
>> > Anyway still if you would prefer to remove these then only 2 sections will
>be available in this doc.
>> > Please let us know if it is ok.
>>
>> I think it is better to avoid repetition.
>>
>>
>> > >> >> +*  Multiple paths::
>> > >> >> +
>> > >> >> +       $ CFLAGS=-I/path1 -I/path2 meson build
>> > >> >
>> > >> > Some quotes are missing to set multiple paths.
>> >
>> > Is <build_dir>  meant here?
>>
>> I am just saying that space-separated value require some quotes.
>>
>> > >> >> +Below are some examples that show how to export libraries and
>> > >> >> +their header paths.
>> > >> >> +
>> > >> >> +To specify a single library at a time::
>> > >> >> +
>> > >> >> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
>> > >> >> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
>> > >> >> +
>> > >> >> +To specify multiple libraries at a time::
>> > >> >> +
>> > >> >> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
>> > >> >> +    $ CFLAGS=-I/path/zuc/include \
>> > >> >> +             -I/path/libsso_kasumi/include \
>> > >> >> +         meson build
>> > >> >
>> > >> > Why export is used for LIBRARY_PATH and not CFLAGS?
>> > >> > I think both variables can be exported or prepend the meson
>command?
>> >
>> > CFLAGS given in meson command works and  also CFLAGS can be
>exported.
>> > LIBRARY_PATH  cannot be prepended to meson command. We tried but it
>is not reflecting the required values.
>> > Environment variables set using export is considered and not as command
>line args of meson command.
>>
>> Please we need to understand why LIBRARY_PATH is not working when
>> preprended in the meson command.
>> Do you have more informations?
>>
>
>Rather than using the environment, one can also use "-Dc_args='...'
>-Dc_link_args='...'" when calling meson.
>
>Otherwise, I agree with Thomas, we should find out why the LIBRARY_PATH
>doesn't work when passed in directly. I'm not aware of any reason it shouldn't
>work.
>
>/Bruce

As per our earlier analysis,  

LDFLAGS was not supported earlier in meson command.

find_library() is used to check whether the external dependency library exists or not in order to configure the build.
This function uses the standard system path to check existence of library, or else LIBRARY_PATH can be used as environment variable for custom paths.
LIBRARY_PATH is not supported directly (not a build configure option).

Only recently we can observe that LDFLAGS support is included in meson and linker options can be provided using LDFLAGS.
$ CFLAGS=-fsomething LDFLAGS=-Wl,--linker-flag meson <options>

Although LDFLAGS can be used and build configuration gets updated in LINK_ARGS,

'dependencies' option of static_library and shared_library() should be an object which is return value from dependency()/find_library().
Due to use of find_library() currently, we need to set the environment variable.

Regards
M.P.Jananee

[dpdk-dev] [PATCH v7] doc: add meson ut info in prog guide

[Agalya Babu RadhaKrishnan]

From: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
v7: Updated v6 patch comments
v6: Updated comments
v5: Modified
v4: Typos corrected
v3: Modified
v2: Removed enhancement details
---
 .../prog_guide/build-sdk-meson.rst}           |  7 +-
 doc/guides/prog_guide/index.rst               |  2 +
 doc/guides/prog_guide/meson_ut.rst            | 92 +++++++++++++++++++
 3 files changed, 98 insertions(+), 3 deletions(-)
 rename doc/{build-sdk-meson.txt => guides/prog_guide/build-sdk-meson.rst} (97%)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/build-sdk-meson.txt b/doc/guides/prog_guide/build-sdk-meson.rst
similarity index 97%
rename from doc/build-sdk-meson.txt
rename to doc/guides/prog_guide/build-sdk-meson.rst
index fc7fe37b5..34c363694 100644
--- a/doc/build-sdk-meson.txt
+++ b/doc/guides/prog_guide/build-sdk-meson.rst
@@ -1,5 +1,5 @@
-INSTALLING DPDK USING THE MESON BUILD SYSTEM
----------------------------------------------
+Installing DPDK Using the meson build system
+============================================
 
 Summary
 --------
@@ -162,7 +162,8 @@ command::
 
 For example if the target machine is arm64 we can use the following
 command::
-	meson arm-build --cross-file config/arm/arm64_armv8_linux_gcc
+
+        meson arm-build --cross-file config/arm/arm64_armv8_linux_gcc
 
 where config/arm/arm64_armv8_linux_gcc contains settings for the compilers
 and other build tools to be used, as well as characteristics of the target
diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 692409af8..0bab96c58 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -60,6 +60,8 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    build-sdk-meson
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..18392ce04
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,92 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+    Copyright(c) 2018-2019 Intel Corporation.
+
+Running DPDK Unit Tests with Meson
+==================================
+
+This section describes how to run test cases with the DPDK meson build system.
+
+Steps to build and run unit test cases using meson can be referred
+in :doc:`build-sdk-meson`
+
+Grouping of test cases
+----------------------
+
+Testcases have been grouped into four different groups.
+
+* Fast tests.
+* Performance tests.
+* Driver tests.
+* Tests which produce lists of objects as output, and therefore that need
+  manual checking.
+
+Testcases can be run in parallel or non-parallel mode using the ``is_parallel`` argument
+of ``test()`` in meson.build
+
+These tests can be run using the argument to ``meson test`` as
+``--suite project_name:label``.
+
+For example::
+
+    $ meson test --suite DPDK:fast-tests
+
+The project name is optional so the following is equivalent to the previous
+command::
+
+    $ meson test --suite fast-tests
+
+The meson command to list all available tests::
+
+    $ meson test --list
+
+
+Dealing with skipped test cases
+-------------------------------
+
+Some unit test cases have a dependency on external libraries, driver modules
+or config flags, without which the test cases cannot be run. Such test cases
+will be reported as skipped if they cannot run. To enable those test cases,
+the user should ensure the required dependencies are met.  Below are a few
+possible causes why tests may be skipped and how they may be resolved:
+
+#. Optional external libraries are not found.
+#. Config flags for the dependent library are not enabled.
+#. Dependent driver modules are not installed on the system.
+
+To help find missing libraries, the user can specify additional search paths
+for those libraries as below:
+
+* Single path::
+
+      $ export LIBRARY_PATH=path
+
+* Multiple paths::
+
+      $ export LIBRARY_PATH=path1:path2:path3
+
+Some functionality may be disabled due to library headers being missed as part
+of the build. To specify an additional search path for headers at
+configuration time, use one of the commands below:
+
+*  Single path::
+
+       $ CFLAGS=-I/path meson build
+
+*  Multiple paths::
+
+       $ CFLAGS=`-I/path1 -I/path2 meson build`
+
+Below are some examples that show how to export libraries and their header
+paths.
+
+To specify a single library at a time::
+
+    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
+    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
+
+To specify multiple libraries at a time::
+
+    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
+    $ CFLAGS=-I/path/zuc/include \
+             -I/path/libsso_kasumi/include \
+	     meson build
-- 
2.17.2

Re: [dpdk-dev] [PATCH v7] doc: add meson ut info in prog guide

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Agalya Babu
> RadhaKrishnan
> Sent: Wednesday, August 7, 2019 7:26 PM
> To: dev@dpdk.org
> Cc: reshma.pattan@intel.com; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; bruce.richardson@intel.com;
> jananeex.m.parthasarathy@intel.com; Hari Kumar Vemula
> <hari.kumarx.vemula@intel.com>
> Subject: [dpdk-dev] [PATCH v7] doc: add meson ut info in prog guide
> 
> From: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> 
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> v7: Updated v6 patch comments
> v6: Updated comments
> v5: Modified
> v4: Typos corrected
> v3: Modified
> v2: Removed enhancement details
> ---
> +
> +Testcases can be run in parallel or non-parallel mode using the
> +``is_parallel`` argument of ``test()`` in meson.build
> +
> +These tests can be run using the argument to ``meson test`` as
> +``--suite project_name:label``.
> +
> +For example::
> +
> +    $ meson test --suite DPDK:fast-tests
> +
> +The project name is optional so the following is equivalent to the
> +previous
> +command::
> +
> +    $ meson test --suite fast-tests

I think, It is good mention about the timeout and/or other valid(if any) UT parameter as well.

Re: [dpdk-dev] [PATCH v7] doc: add meson ut info in prog guide

[Michael Santana Francisco]

On 8/7/19 9:56 AM, Agalya Babu RadhaKrishnan wrote:
> From: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
>
> Add a programmer's guide section for meson ut
>
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> v7: Updated v6 patch comments
> v6: Updated comments
> v5: Modified
> v4: Typos corrected
> v3: Modified
> v2: Removed enhancement details
> ---
>   .../prog_guide/build-sdk-meson.rst}           |  7 +-
>   doc/guides/prog_guide/index.rst               |  2 +
>   doc/guides/prog_guide/meson_ut.rst            | 92 +++++++++++++++++++
>   3 files changed, 98 insertions(+), 3 deletions(-)
>   rename doc/{build-sdk-meson.txt => guides/prog_guide/build-sdk-meson.rst} (97%)
>   create mode 100644 doc/guides/prog_guide/meson_ut.rst
>
> diff --git a/doc/build-sdk-meson.txt b/doc/guides/prog_guide/build-sdk-meson.rst
> similarity index 97%
> rename from doc/build-sdk-meson.txt
> rename to doc/guides/prog_guide/build-sdk-meson.rst
> index fc7fe37b5..34c363694 100644
> --- a/doc/build-sdk-meson.txt
> +++ b/doc/guides/prog_guide/build-sdk-meson.rst
> @@ -1,5 +1,5 @@
> -INSTALLING DPDK USING THE MESON BUILD SYSTEM
> ----------------------------------------------
> +Installing DPDK Using the meson build system
> +============================================
>   
>   Summary
>   --------
> @@ -162,7 +162,8 @@ command::
>   
>   For example if the target machine is arm64 we can use the following
>   command::
> -	meson arm-build --cross-file config/arm/arm64_armv8_linux_gcc
> +
> +        meson arm-build --cross-file config/arm/arm64_armv8_linux_gcc
>   
>   where config/arm/arm64_armv8_linux_gcc contains settings for the compilers
>   and other build tools to be used, as well as characteristics of the target
> diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
> index 692409af8..0bab96c58 100644
> --- a/doc/guides/prog_guide/index.rst
> +++ b/doc/guides/prog_guide/index.rst
> @@ -60,6 +60,8 @@ Programmer's Guide
>       source_org
>       dev_kit_build_system
>       dev_kit_root_make_help
> +    build-sdk-meson
> +    meson_ut
>       extend_dpdk
>       build_app
>       ext_app_lib_make_help
> diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
> new file mode 100644
> index 000000000..18392ce04
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,92 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +    Copyright(c) 2018-2019 Intel Corporation.
> +
> +Running DPDK Unit Tests with Meson
> +==================================
> +
> +This section describes how to run test cases with the DPDK meson build system.
> +
> +Steps to build and run unit test cases using meson can be referred
> +in :doc:`build-sdk-meson`
I think this is misleading. As far as I can see build-sdk-meson doesn't 
provide any details as to how to "run" test cases. I think you meant to 
say build-sdk-meson has the steps to build dpdk, or something among 
those lines
> +
> +Grouping of test cases
> +----------------------
> +
> +Testcases have been grouped into four different groups.

space between Test and cases.

I prefer not having to use the word 'group' twice in the same sentence, 
it just sounds awkward to me. I would do s/different groups/different 
test suites/

> +
> +* Fast tests.
> +* Performance tests.
> +* Driver tests.
> +* Tests which produce lists of objects as output, and therefore that need
> +  manual checking.
> +
> +Testcases can be run in parallel or non-parallel mode using the ``is_parallel`` argument

space between Test and cases.

I would also mention that by default all tests run serially for better 
stability

> +of ``test()`` in meson.build
> +
> +These tests can be run using the argument to ``meson test`` as
> +``--suite project_name:label``.
> +
> +For example::
> +
> +    $ meson test --suite DPDK:fast-tests

I would also somehow mention that these examples only work when your 
current working directory is the build directory. If you are not in the 
build directory you need to pass the -C flag, so like:

$ meson test -C <build path> --suite DPDK:fast-tests

the build path can either be a relative path or an absolute path, but I 
would skip this detail

> +
> +The project name is optional so the following is equivalent to the previous
> +command::
> +
> +    $ meson test --suite fast-tests
> +
> +The meson command to list all available tests::
> +
> +    $ meson test --list
> +
> +
> +Dealing with skipped test cases
> +-------------------------------
> +
> +Some unit test cases have a dependency on external libraries, driver modules
> +or config flags, without which the test cases cannot be run. Such test cases
> +will be reported as skipped if they cannot run. To enable those test cases,
> +the user should ensure the required dependencies are met.  Below are a few
> +possible causes why tests may be skipped and how they may be resolved:
> +
> +#. Optional external libraries are not found.
> +#. Config flags for the dependent library are not enabled.
> +#. Dependent driver modules are not installed on the system.

#. Note enough processing cores. Some tests are skipped on machines with 
2 or 4 cores


The rest lgtm

Acked-by: Michael Santana <msantana@redhat.com <mailto:msantana@redhat.com>>

> +
> +To help find missing libraries, the user can specify additional search paths
> +for those libraries as below:
> +
> +* Single path::
> +
> +      $ export LIBRARY_PATH=path
> +
> +* Multiple paths::
> +
> +      $ export LIBRARY_PATH=path1:path2:path3
> +
> +Some functionality may be disabled due to library headers being missed as part
> +of the build. To specify an additional search path for headers at
> +configuration time, use one of the commands below:
> +
> +*  Single path::
> +
> +       $ CFLAGS=-I/path meson build
> +
> +*  Multiple paths::
> +
> +       $ CFLAGS=`-I/path1 -I/path2 meson build`
> +
> +Below are some examples that show how to export libraries and their header
> +paths.
> +
> +To specify a single library at a time::
> +
> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
> +
> +To specify multiple libraries at a time::
> +
> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
> +    $ CFLAGS=-I/path/zuc/include \
> +             -I/path/libsso_kasumi/include \
> +	     meson build

[dpdk-dev] [PATCH v8] doc: add meson ut info in prog guide

[Jananee Parthasarathy]

From: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>

Add a programmer's guide section for meson ut

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Michael Santana <msantana@redhat.com>
---
v8: Addressed v7 patch comments
v7: Updated v6 patch comments
v6: Updated comments
v5: Modified
v4: Typos corrected
v3: Modified
v2: Removed enhancement details
---
 .../prog_guide/build-sdk-meson.rst}           |   7 +-
 doc/guides/prog_guide/index.rst               |   2 +
 doc/guides/prog_guide/meson_ut.rst            | 104 ++++++++++++++++++
 3 files changed, 110 insertions(+), 3 deletions(-)
 rename doc/{build-sdk-meson.txt => guides/prog_guide/build-sdk-meson.rst} (97%)
 create mode 100644 doc/guides/prog_guide/meson_ut.rst

diff --git a/doc/build-sdk-meson.txt b/doc/guides/prog_guide/build-sdk-meson.rst
similarity index 97%
rename from doc/build-sdk-meson.txt
rename to doc/guides/prog_guide/build-sdk-meson.rst
index fc7fe37b5..34c363694 100644
--- a/doc/build-sdk-meson.txt
+++ b/doc/guides/prog_guide/build-sdk-meson.rst
@@ -1,5 +1,5 @@
-INSTALLING DPDK USING THE MESON BUILD SYSTEM
----------------------------------------------
+Installing DPDK Using the meson build system
+============================================
 
 Summary
 --------
@@ -162,7 +162,8 @@ command::
 
 For example if the target machine is arm64 we can use the following
 command::
-	meson arm-build --cross-file config/arm/arm64_armv8_linux_gcc
+
+        meson arm-build --cross-file config/arm/arm64_armv8_linux_gcc
 
 where config/arm/arm64_armv8_linux_gcc contains settings for the compilers
 and other build tools to be used, as well as characteristics of the target
diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 692409af8..0bab96c58 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -60,6 +60,8 @@ Programmer's Guide
     source_org
     dev_kit_build_system
     dev_kit_root_make_help
+    build-sdk-meson
+    meson_ut
     extend_dpdk
     build_app
     ext_app_lib_make_help
diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
new file mode 100644
index 000000000..45193ddde
--- /dev/null
+++ b/doc/guides/prog_guide/meson_ut.rst
@@ -0,0 +1,104 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+    Copyright(c) 2018-2019 Intel Corporation.
+
+Running DPDK Unit Tests with Meson
+==================================
+
+This section describes how to run test cases with the DPDK meson build system.
+
+Steps to build and install DPDK using meson can be referred
+in :doc:`build-sdk-meson`
+
+Grouping of test cases
+----------------------
+
+Test cases have been classified into four different groups.
+
+* Fast tests.
+* Performance tests.
+* Driver tests.
+* Tests which produce lists of objects as output, and therefore that need
+  manual checking.
+
+These tests can be run using the argument to ``meson test`` as
+``--suite project_name:label``.
+
+For example::
+
+    $ meson test -C <build path> --suite DPDK:fast-tests
+
+If the ``<build path>`` is current working directory,
+the ``-C <build path>`` option can be skipped as below::
+
+    $ meson test --suite DPDK:fast-tests
+
+The project name is optional so the following is equivalent to the previous
+command::
+
+    $ meson test --suite fast-tests
+
+The meson command to list all available tests::
+
+    $ meson test --list
+
+Test cases are run serially by default for better stability.
+
+Arguments of ``test()`` that can be provided in meson.build are as below:
+
+* ``is_parallel`` is used to run test case either in parallel or non-parallel mode.
+* ``timeout`` is used to specify the timeout of test case.
+* ``args`` is used to specify test specific parameters.
+* ``env`` is used to specify test specific environment parameters.
+
+
+Dealing with skipped test cases
+-------------------------------
+
+Some unit test cases have a dependency on external libraries, driver modules
+or config flags, without which the test cases cannot be run. Such test cases
+will be reported as skipped if they cannot run. To enable those test cases,
+the user should ensure the required dependencies are met.  Below are a few
+possible causes why tests may be skipped and how they may be resolved:
+
+#. Optional external libraries are not found.
+#. Config flags for the dependent library are not enabled.
+#. Dependent driver modules are not installed on the system.
+#. Not enough processing cores. Some tests are skipped on machines with 2 or 4 cores.
+
+To help find missing libraries, the user can specify additional search paths
+for those libraries as below:
+
+* Single path::
+
+      $ export LIBRARY_PATH=path
+
+* Multiple paths::
+
+      $ export LIBRARY_PATH=path1:path2:path3
+
+Some functionality may be disabled due to library headers being missed as part
+of the build. To specify an additional search path for headers at
+configuration time, use one of the commands below:
+
+*  Single path::
+
+       $ CFLAGS=-I/path meson build
+
+*  Multiple paths::
+
+       $ CFLAGS=`-I/path1 -I/path2 meson build`
+
+Below are some examples that show how to export libraries and their header
+paths.
+
+To specify a single library at a time::
+
+    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
+    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
+
+To specify multiple libraries at a time::
+
+    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
+    $ CFLAGS=-I/path/zuc/include \
+             -I/path/libsso_kasumi/include \
+	     meson build
-- 
2.17.2

Re: [dpdk-dev] [PATCH v8] doc: add meson ut info in prog guide

[Thomas Monjalon]

12/08/2019 14:40, Jananee Parthasarathy:
> From: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> 
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Michael Santana <msantana@redhat.com>

The last part about enabling some skipped test case is not correct.
Because this patch is very old, I apply it without this last part:

> +To help find missing libraries, the user can specify additional search paths
> +for those libraries as below:
> +
> +* Single path::
> +
> +      $ export LIBRARY_PATH=path
> +
> +* Multiple paths::
> +
> +      $ export LIBRARY_PATH=path1:path2:path3
> +
> +Some functionality may be disabled due to library headers being missed as part
> +of the build. To specify an additional search path for headers at
> +configuration time, use one of the commands below:
> +
> +*  Single path::
> +
> +       $ CFLAGS=-I/path meson build
> +
> +*  Multiple paths::
> +
> +       $ CFLAGS=`-I/path1 -I/path2 meson build`
> +
> +Below are some examples that show how to export libraries and their header
> +paths.
> +
> +To specify a single library at a time::
> +
> +    $ export LIBRARY_PATH=/root/wireless_libs/zuc/
> +    $ CFLAGS=-I/root/wireless_libs/zuc/include meson build
> +
> +To specify multiple libraries at a time::
> +
> +    $ export LIBRARY_PATH=/path/zuc/:/path/libsso_kasumi/build/
> +    $ CFLAGS=-I/path/zuc/include \
> +             -I/path/libsso_kasumi/include \
> +	     meson build

[dpdk-dev] [PATCH v2] eal: fix core number validation

[Hari kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

--
v2: Replace strtoul with strtol
    Modified log message
--

Signed-off-by: Hari kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..b24668c23 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,7 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -603,6 +603,11 @@ eal_parse_corelist(const char *corelist)
 			max = idx;
 			if (min == RTE_MAX_LCORE)
 				min = idx;
+			if ((unsigned int)idx >= cfg->lcore_count ||
+					(unsigned int)min >= cfg->lcore_count) {
+				return -1;
+			}
+
 			for (idx = min; idx <= max; idx++) {
 				if (cfg->lcore_role[idx] != ROLE_RTE) {
 					cfg->lcore_role[idx] = ROLE_RTE;
@@ -1103,6 +1108,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1151,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number given core range should be(0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
-- 
2.17.2

Re: [dpdk-dev] [PATCH v2] eal: fix core number validation

[David Marchand]

On Thu, Jan 3, 2019 at 1:28 PM Hari kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
>
> Added valid range checks to fix the crash.
>
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
>
> --
> v2: Replace strtoul with strtol
>     Modified log message
>

You did not take my other comments into account and the ut updates are
missing.


-- 
David Marchand

[dpdk-dev] [PATCH v2] eal: fix core number validation

[Hari Kumar Vemula]

From: Hari kumar Vemula <hari.kumarx.vemula@intel.com>

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 10 ++++++++--
 test/test/test_eal_flags.c                 | 14 +++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..4a900c548 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+			if (idx < 0 || idx >= (int)cfg->lcore_count)
+				return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -603,6 +605,7 @@ eal_parse_corelist(const char *corelist)
 			max = idx;
 			if (min == RTE_MAX_LCORE)
 				min = idx;
 			for (idx = min; idx <= max; idx++) {
 				if (cfg->lcore_role[idx] != ROLE_RTE) {
 					cfg->lcore_role[idx] = ROLE_RTE;
@@ -1103,6 +1106,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1149,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number, core range should be (0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..ec79b2242 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -513,6 +513,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* when core number is negative value*/
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* when core number is maximum value*/
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "999999999" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-9999999" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +566,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

[dpdk-dev] [PATCH v3] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

--
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
--

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..9431c024e 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+			if (idx < 0 || idx >= (int)cfg->lcore_count)
+				return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"Invalid core number, core range should be (0, %u)\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..28e68a6c9 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -513,6 +513,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "999999999" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-9999999" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +566,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-dev] [PATCH v3] eal: fix core number validation

[David Marchand]

Hello Hari,

On Mon, Jan 7, 2019 at 11:26 AM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
>
> Added valid range checks to fix the crash.
>
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
>
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
>
> --
> v3: Added unit test cases for invalid core number range
> v2: Replace strtoul with strtol
>     Modified log message
> --
>
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>  lib/librte_eal/common/eal_common_options.c |  9 +++++++--
>  test/test/test_eal_flags.c                 | 14 +++++++++++++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 6e3a83b98..9431c024e 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
>                 if (*corelist == '\0')
>                         return -1;
>                 errno = 0;
> -               idx = strtoul(corelist, &end, 10);
> +               idx = strtol(corelist, &end, 10);
> +                       if (idx < 0 || idx >= (int)cfg->lcore_count)
> +                               return -1;
>                 if (errno || end == NULL)
>                         return -1;
>                 while (isblank(*end))
>

Please fix indentation.


> @@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
>  {
>         static int b_used;
>         static int w_used;
> +       struct rte_config *cfg = rte_eal_get_configuration();
>
>         switch (opt) {
>         /* blacklist */
> @@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
>         /* corelist */
>         case 'l':
>                 if (eal_parse_corelist(optarg) < 0) {
> -                       RTE_LOG(ERR, EAL, "invalid core list\n");
> +                       RTE_LOG(ERR, EAL,
> +                               "Invalid core number, core range should be
> (0, %u)\n",
> +                                       cfg->lcore_count-1);
>                         return -1;
>                 }
>
>
eal_parse_corelist can error for both invalid core number but also for
incorrectly formatted input.
How about "invalid core list, please check core numbers are in [0, %u]
range" ?

diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..28e68a6c9 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -513,6 +513,16 @@ test_missing_c_flag(void)
>         const char *argv25[] = { prgname, prefix, mp_flag,
>                                  "-n", "3", "--lcores",
>                                  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
> +       /* core number is negative value */
> +       const char * const argv26[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5" };
> +       const char * const argv27[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5-7" };
>
+       /* core number is maximum value */
> +       const char * const argv28[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "999999999" };
> +       const char * const argv29[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "1-9999999" };
>

Well, the maximum value is not really "999999999".
Please check against RTE_MAX_LCORE.


-- 
David Marchand

[dpdk-dev] [PATCH v4] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
--
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..fc45bf953 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -513,6 +514,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "RTE_MAX_LCORE+1" };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "--lcores", "1-(RTE_MAX_LCORE+1)" };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +567,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-dev] [PATCH v4] eal: fix core number validation

[David Marchand]

On Fri, Jan 11, 2019 at 3:15 PM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

>
> diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..fc45bf953 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -18,6 +18,7 @@
>  #include <sys/file.h>
>  #include <limits.h>
>
> +#include <rte_per_lcore.h>
>  #include <rte_debug.h>
>  #include <rte_string_fns.h>
>
> @@ -513,6 +514,16 @@ test_missing_c_flag(void)
>         const char *argv25[] = { prgname, prefix, mp_flag,
>                                  "-n", "3", "--lcores",
>                                  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
>
+       /* core number is negative value */
> +       const char * const argv26[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5" };
> +       const char * const argv27[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "-5-7" };
>

I did not see this before, but you fixed the "-l" eal option, not
"--lcores" option.
So those unit tests are wrong.



> +       /* core number is maximum value */
> +       const char * const argv28[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores", "RTE_MAX_LCORE+1" };
> +       const char * const argv29[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "--lcores",
> "1-(RTE_MAX_LCORE+1)" };
>
>         if (launch_proc(argv2) != 0) {
>                 printf("Error - "
>

Passing "RTE_MAX_LCORE+1" is indeed wrong (be it with "-l" or "--lcores"
options), but I would still prefer to check the formatted value of
RTE_MAX_LCORE (no need for that +1, btw).
So please, in next version, test against "-l", RTE_STR(RTE_MAX_LCORE) and
"-l", "1-" RTE_STR(RTE_MAX_LCORE).


Thanks.

-- 
David Marchand

[dpdk-dev] [PATCH v5] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
--
v5: corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++++++--
 test/test/test_eal_flags.c                 | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..4dc22ec36 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -513,6 +514,16 @@ test_missing_c_flag(void)
 	const char *argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+	/* core number is negative value */
+	const char * const argv26[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5" };
+	const char * const argv27[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv28[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+	const char * const argv29[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 
 	if (launch_proc(argv2) != 0) {
 		printf("Error - "
@@ -556,7 +567,9 @@ test_missing_c_flag(void)
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
 	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-dev] [PATCH v5] eal: fix core number validation

[David Marchand]

On Mon, Jan 14, 2019 at 11:31 AM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..4dc22ec36 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -18,6 +18,7 @@
>  #include <sys/file.h>
>  #include <limits.h>
>
> +#include <rte_per_lcore.h>
>  #include <rte_debug.h>
>  #include <rte_string_fns.h>
>
> @@ -513,6 +514,16 @@ test_missing_c_flag(void)
>         const char *argv25[] = { prgname, prefix, mp_flag,
>                                  "-n", "3", "--lcores",
>                                  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
> +       /* core number is negative value */
> +       const char * const argv26[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l", "-5" };
> +       const char * const argv27[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l", "-5-7" };
> +       /* core number is maximum value */
> +       const char * const argv28[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
> +       const char * const argv29[] = { prgname, prefix, mp_flag,
> +                               "-n", "3", "-l",
> "1-"RTE_STR(RTE_MAX_LCORE) };
>

Please move this block with the other "-l" options.
You can add my Reviewed-by tag with this.


Thanks.

-- 
David Marchand

[dpdk-dev] [PATCH v6] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
v6: Changed testcase order
v5: Corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
    Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++-
 test/test/test_eal_flags.c                 | 61 ++++++++++++++--------
 2 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
 		if (*corelist == '\0')
 			return -1;
 		errno = 0;
-		idx = strtoul(corelist, &end, 10);
+		idx = strtol(corelist, &end, 10);
+		if (idx < 0 || idx >= (int)cfg->lcore_count)
+			return -1;
 		if (errno || end == NULL)
 			return -1;
 		while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
 	static int b_used;
 	static int w_used;
+	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
 	/* corelist */
 	case 'l':
 		if (eal_parse_corelist(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid core list\n");
+			RTE_LOG(ERR, EAL,
+				"invalid core list, please check core numbers are in [0, %u] range\n",
+					cfg->lcore_count-1);
 			return -1;
 		}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..e3a60c7ae 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include <sys/file.h>
 #include <limits.h>
 
+#include <rte_per_lcore.h>
 #include <rte_debug.h>
 #include <rte_string_fns.h>
 
@@ -477,40 +478,50 @@ test_missing_c_flag(void)
 				"-n", "3", "-l", "1," };
 	const char *argv10[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "-l", "1#2" };
+	/* core number is negative value */
+	const char * const argv11[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5" };
+	const char * const argv12[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "-5-7" };
+	/* core number is maximum value */
+	const char * const argv13[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+	const char * const argv14[] = { prgname, prefix, mp_flag,
+				"-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 	/* sanity check test - valid corelist value */
-	const char *argv11[] = { prgname, prefix, mp_flag,
+	const char * const argv15[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "-l", "1-2,3" };
 
 	/* --lcores flag but no lcores value */
-	const char *argv12[] = { prgname, prefix, mp_flag,
+	const char * const argv16[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores" };
-	const char *argv13[] = { prgname, prefix, mp_flag,
+	const char * const argv17[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", " " };
 	/* bad lcores value */
-	const char *argv14[] = { prgname, prefix, mp_flag,
+	const char * const argv18[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "1-3-5" };
-	const char *argv15[] = { prgname, prefix, mp_flag,
+	const char * const argv19[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "0-1,,2" };
-	const char *argv16[] = { prgname, prefix, mp_flag,
+	const char * const argv20[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "0-,1" };
-	const char *argv17[] = { prgname, prefix, mp_flag,
+	const char * const argv21[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(0-,2-4)" };
-	const char *argv18[] = { prgname, prefix, mp_flag,
+	const char * const argv22[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(-1,2)" };
-	const char *argv19[] = { prgname, prefix, mp_flag,
+	const char * const argv23[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(2-4)@(2-4-6)" };
-	const char *argv20[] = { prgname, prefix, mp_flag,
+	const char * const argv24[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(a,2)" };
-	const char *argv21[] = { prgname, prefix, mp_flag,
+	const char * const argv25[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "1-3@(1,3)" };
-	const char *argv22[] = { prgname, prefix, mp_flag,
+	const char * const argv26[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "3@((1,3)" };
-	const char *argv23[] = { prgname, prefix, mp_flag,
+	const char * const argv27[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "(4-7)=(1,3)" };
-	const char *argv24[] = { prgname, prefix, mp_flag,
+	const char * const argv28[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores", "[4-7]@(1,3)" };
 	/* sanity check of tests - valid lcores value */
-	const char *argv25[] = { prgname, prefix, mp_flag,
+	const char * const argv29[] = { prgname, prefix, mp_flag,
 				 "-n", "3", "--lcores",
 				 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
 
@@ -538,31 +549,35 @@ test_missing_c_flag(void)
 			|| launch_proc(argv7) == 0
 			|| launch_proc(argv8) == 0
 			|| launch_proc(argv9) == 0
-			|| launch_proc(argv10) == 0) {
+			|| launch_proc(argv10) == 0
+			|| launch_proc(argv11) == 0
+			|| launch_proc(argv12) == 0
+			|| launch_proc(argv13) == 0
+			|| launch_proc(argv14) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid -l flag\n");
 		return -1;
 	}
-	if (launch_proc(argv11) != 0) {
+	if (launch_proc(argv15) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
 	}
 
 	/* start --lcores tests */
-	if (launch_proc(argv12) == 0 || launch_proc(argv13) == 0 ||
-	    launch_proc(argv14) == 0 || launch_proc(argv15) == 0 ||
-	    launch_proc(argv16) == 0 || launch_proc(argv17) == 0 ||
+	if (launch_proc(argv16) == 0 || launch_proc(argv17) == 0 ||
 	    launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
 	    launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
-	    launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-	    launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+	    launch_proc(argv22) == 0 || launch_proc(argv23) == 0 ||
+	    launch_proc(argv24) == 0 || launch_proc(argv25) == 0 ||
+	    launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+	    launch_proc(argv28) == 0) {
 		printf("Error - "
 		       "process ran without error with invalid --lcore flag\n");
 		return -1;
 	}
 
-	if (launch_proc(argv25) != 0) {
+	if (launch_proc(argv29) != 0) {
 		printf("Error - "
 		       "process did not run ok with valid corelist value\n");
 		return -1;
-- 
2.17.2

Re: [dpdk-dev] [PATCH v6] eal: fix core number validation

[Bruce Richardson]

On Thu, Jan 17, 2019 at 12:13:12PM +0000, Hari Kumar Vemula wrote:
> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
> 
> Added valid range checks to fix the crash.
> 
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
> 
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> --
> v6: Changed testcase order
> v5: Corrected unit test case for -l option
> v4: Used RTE_MAX_LCORE for max core check
> v3: Added unit test cases for invalid core number range
> v2: Replace strtoul with strtol
>     Modified log message
> ---
>  lib/librte_eal/common/eal_common_options.c |  9 +++-
>  test/test/test_eal_flags.c                 | 61 ++++++++++++++--------
>  2 files changed, 45 insertions(+), 25 deletions(-)
> 
Is this patch related to, or does it fix Bugzilla #19?

https://bugs.dpdk.org/show_bug.cgi?id=19

/Bruce

Re: [dpdk-dev] [PATCH v6] eal: fix core number validation

[David Marchand]

On Thu, Jan 17, 2019 at 1:19 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, Jan 17, 2019 at 12:13:12PM +0000, Hari Kumar Vemula wrote:
> > When incorrect core value or range provided,
> > as part of -l command line option, a crash occurs.
> >
> > Added valid range checks to fix the crash.
> >
> > Added ut check for negative core values.
> > Added unit test case for invalid core number range.
> >
> > Fixes: d888cb8b9613 ("eal: add core list input format")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> > Reviewed-by: David Marchand <david.marchand@redhat.com>
> > --
> > v6: Changed testcase order
> > v5: Corrected unit test case for -l option
> > v4: Used RTE_MAX_LCORE for max core check
> > v3: Added unit test cases for invalid core number range
> > v2: Replace strtoul with strtol
> >     Modified log message
> > ---
> >  lib/librte_eal/common/eal_common_options.c |  9 +++-
> >  test/test/test_eal_flags.c                 | 61 ++++++++++++++--------
> >  2 files changed, 45 insertions(+), 25 deletions(-)
> >
> Is this patch related to, or does it fix Bugzilla #19?
>
> https://bugs.dpdk.org/show_bug.cgi?id=19


Separate issue, from my pov.
I would say the issue happens with a dpdk process that has no cpu available
wrt CONFIG_RTE_MAX_CORES.
eal_auto_detect_cores() then removes all cores from cfg->lcore_role[], then
eal_adjust_config() an incorrect master lcore is chosen at
cfg->master_lcore = rte_get_next_lcore(-1, 0, 0); ?


-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [PATCH v6] eal: fix core number validation

[Thomas Monjalon]

17/01/2019 13:13, Hari Kumar Vemula:
> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
> 
> Added valid range checks to fix the crash.
> 
> Added ut check for negative core values.
> Added unit test case for invalid core number range.
> 
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>

Applied, thanks

[dpdk-dev] [PATCH] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

Create bonded device test is failing due to improper initialisation in
bonded device configuration. which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.

Fixes: 2efb58cbab ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..e0888e960 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2225,6 +2225,11 @@ static void
 bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct bond_dev_private *internals = dev->data->dev_private;
+	struct rte_eth_desc_lim bond_lim = {
+		.nb_max = UINT16_MAX,
+		.nb_min = 0,
+		.nb_align = 1,
+	};
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
@@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim = bond_lim;
+	dev_info->tx_desc_lim = bond_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2

Re: [dpdk-dev] [PATCH] net/bonding: fix create bonded device test failure

[Chas Williams]

On 1/7/19 8:01 AM, Hari Kumar Vemula wrote:
> Create bonded device test is failing due to improper initialisation in
> bonded device configuration. which leads to crash while setting up queues.
> 
> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
> bonded device which fails.
> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
> during bond_alloc().
> Hence nb_rx_desc (1024) is > 0 and test fails.
> 
> Fix is to set the default values of rx_desc_lim of bonded device to
> appropriate value.

The default values for the bond device aren't known until the first
slave is added.  Can you explain your setup?  What PMD are you
using for testing?

> 
> Fixes: 2efb58cbab ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 44deaf119..e0888e960 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -2225,6 +2225,11 @@ static void
>   bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   {
>   	struct bond_dev_private *internals = dev->data->dev_private;
> +	struct rte_eth_desc_lim bond_lim = {
> +		.nb_max = UINT16_MAX,
> +		.nb_min = 0,
> +		.nb_align = 1,
> +	};
>   
>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>   	uint16_t max_nb_tx_queues = UINT16_MAX;
> @@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>   	       sizeof(dev_info->default_txconf));
>   
> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> -	       sizeof(dev_info->rx_desc_lim));
> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> -	       sizeof(dev_info->tx_desc_lim));
> +	dev_info->rx_desc_lim = bond_lim;
> +	dev_info->tx_desc_lim = bond_lim;
>   
>   	/**
>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/bonding: fix create bonded device test failure

[Ferruh Yigit]

On 1/7/2019 6:44 PM, Chas Williams wrote:
> 
> 
> On 1/7/19 8:01 AM, Hari Kumar Vemula wrote:
>> Create bonded device test is failing due to improper initialisation in
>> bonded device configuration. which leads to crash while setting up queues.
>>
>> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
>> bonded device which fails.
>> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
>> during bond_alloc().
>> Hence nb_rx_desc (1024) is > 0 and test fails.
>>
>> Fix is to set the default values of rx_desc_lim of bonded device to
>> appropriate value.
> 
> The default values for the bond device aren't known until the first
> slave is added.  Can you explain your setup?  What PMD are you
> using for testing?

As far as I understand, 'rte_eth_rx_queue_setup()' is failing with bond eth
device since 'nb_rx_desc' should be less than 'dev_info.rx_desc_lim.nb_max' but
bonding sets 0 to 'nb_max'.

But not sure how to handle this from bonding point of view, this patch gives
most permissive values, but should bonding get these values from its slaves?

> 
>>
>> Fixes: 2efb58cbab ("bond: new link bonding library")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
>> ---
>>   drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
>> index 44deaf119..e0888e960 100644
>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>> @@ -2225,6 +2225,11 @@ static void
>>   bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>>   {
>>   	struct bond_dev_private *internals = dev->data->dev_private;
>> +	struct rte_eth_desc_lim bond_lim = {
>> +		.nb_max = UINT16_MAX,
>> +		.nb_min = 0,
>> +		.nb_align = 1,
>> +	};
>>   
>>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>>   	uint16_t max_nb_tx_queues = UINT16_MAX;
>> @@ -2263,10 +2268,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>>   	       sizeof(dev_info->default_txconf));
>>   
>> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
>> -	       sizeof(dev_info->rx_desc_lim));
>> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
>> -	       sizeof(dev_info->tx_desc_lim));
>> +	dev_info->rx_desc_lim = bond_lim;
>> +	dev_info->tx_desc_lim = bond_lim;
>>   
>>   	/**
>>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
>>

Re: [dpdk-dev] [PATCH] net/bonding: fix create bonded device test failure

[Vemula, Hari KumarX]

Hi Williams,


> -----Original Message-----
> From: Chas Williams [mailto:3chas3@gmail.com]
> Sent: Tuesday, January 8, 2019 12:15 AM
> To: Vemula, Hari KumarX <hari.kumarx.vemula@intel.com>; dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Parthasarathy, JananeeX M
> <jananeex.m.parthasarathy@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix create bonded device test
> failure
> 
> 
> 
> On 1/7/19 8:01 AM, Hari Kumar Vemula wrote:
> > Create bonded device test is failing due to improper initialisation in
> > bonded device configuration. which leads to crash while setting up queues.
> >
> > The value of nb_rx_desc is checked if it is not in range of
> > rx_desc_lim of bonded device which fails.
> > This is due to "rx_desc_lim" is set to 0 as default value of bonded
> > device during bond_alloc().
> > Hence nb_rx_desc (1024) is > 0 and test fails.
> >
> > Fix is to set the default values of rx_desc_lim of bonded device to
> > appropriate value.
> 
> The default values for the bond device aren't known until the first slave is
> added.  Can you explain your setup?  What PMD are you using for testing?
> 
We ran link_bonding_autotest in test application and was failing the test_create_bonded_device as in below log.
Slaves are added during test suite setup and then bonded device is created as per ut.

./x86_64-native-linuxapp-clang/app/test -n 1 -c f
RTE>>link_bonding_autotest

+ ------------------------------------------------------- +
 + Test Suite : Link Bonding Unit Test Suite
 + ------------------------------------------------------- +
Invalid value for nb_rx_desc(=1024), should be: <= 0, = 0, and a product of 0
 + TestCase [ 0] : test_create_bonded_device failed

> >
> > Fixes: 2efb58cbab ("bond: new link bonding library")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> > ---
> >   drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
> >   1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
> > b/drivers/net/bonding/rte_eth_bond_pmd.c
> > index 44deaf119..e0888e960 100644
> > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> > @@ -2225,6 +2225,11 @@ static void
> >   bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info
> *dev_info)
> >   {
> >   	struct bond_dev_private *internals = dev->data->dev_private;
> > +	struct rte_eth_desc_lim bond_lim = {
> > +		.nb_max = UINT16_MAX,
> > +		.nb_min = 0,
> > +		.nb_align = 1,
> > +	};
> >
> >   	uint16_t max_nb_rx_queues = UINT16_MAX;
> >   	uint16_t max_nb_tx_queues = UINT16_MAX; @@ -2263,10 +2268,8
> @@
> > bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info
> *dev_info)
> >   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
> >   	       sizeof(dev_info->default_txconf));
> >
> > -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> > -	       sizeof(dev_info->rx_desc_lim));
> > -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> > -	       sizeof(dev_info->tx_desc_lim));
> > +	dev_info->rx_desc_lim = bond_lim;
> > +	dev_info->tx_desc_lim = bond_lim;
> >
> >   	/**
> >   	 * If dedicated hw queues enabled for link bonding device in LACP
> > mode
> >


Thanks,
Hari.
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [dpdk-dev] [PATCH] net/bonding: fix create bonded device test failure

[Pattan, Reshma]

> -----Original Message-----
> From: Vemula, Hari KumarX
> Sent: Monday, January 7, 2019 1:01 PM
> 
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> -	       sizeof(dev_info->rx_desc_lim));
> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> -	       sizeof(dev_info->tx_desc_lim));
> +	dev_info->rx_desc_lim = bond_lim;
> +	dev_info->tx_desc_lim = bond_lim;


I relooked into this, these values should be filled from salve data like the way done for max_nb_rx_queues and max_nb_tx_queues
Existing code snippet:
if (slave_info.max_rx_queues < max_nb_rx_queues)
                          max_nb_rx_queues = slave_info.max_rx_queues;

 if (slave_info.max_tx_queues < max_nb_tx_queues)
                        max_nb_tx_queues = slave_info.max_tx_queues;

So, something like below you should add for rx/tx desc limit I guess.

if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
                                max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;

if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
                                max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;

dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
        dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;

@Williams/Declan: Does this make sense?

Thanks,
Reshma

[dpdk-dev] [PATCH v2] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

Create bonded device test is failing due to improper initialisation in
bonded device configuration. which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
---
v2: bonded device desc_lim values are received from slave configuration
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..23cec2549 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2228,6 +2228,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
+	uint16_t max_rx_desc_lim = UINT16_MAX;
+	uint16_t max_tx_desc_lim = UINT16_MAX;
 
 	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
 
@@ -2252,6 +2254,12 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 			if (slave_info.max_tx_queues < max_nb_tx_queues)
 				max_nb_tx_queues = slave_info.max_tx_queues;
+
+			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
+				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
+
+			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
+				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
 		}
 	}
 
@@ -2263,10 +2271,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
+	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2

Re: [dpdk-dev] [PATCH v2] net/bonding: fix create bonded device test failure

[Chas Williams]

On 1/28/19 2:28 AM, Hari Kumar Vemula wrote:
> Create bonded device test is failing due to improper initialisation in
> bonded device configuration. which leads to crash while setting up queues.
> 
> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
> bonded device which fails.
> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
> during bond_alloc().
> Hence nb_rx_desc (1024) is > 0 and test fails.
> 
> Fix is to set the default values of rx_desc_lim of bonded device to
> appropriate value.
> 
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>

Acked-by: Chas Williams <chas3@att.com>

> ---
> v2: bonded device desc_lim values are received from slave configuration
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 44deaf119..23cec2549 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -2228,6 +2228,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   
>   	uint16_t max_nb_rx_queues = UINT16_MAX;
>   	uint16_t max_nb_tx_queues = UINT16_MAX;
> +	uint16_t max_rx_desc_lim = UINT16_MAX;
> +	uint16_t max_tx_desc_lim = UINT16_MAX;
>   
>   	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
>   
> @@ -2252,6 +2254,12 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   
>   			if (slave_info.max_tx_queues < max_nb_tx_queues)
>   				max_nb_tx_queues = slave_info.max_tx_queues;
> +
> +			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
> +				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
> +
> +			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
> +				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
>   		}
>   	}
>   
> @@ -2263,10 +2271,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   	memcpy(&dev_info->default_txconf, &internals->default_txconf,
>   	       sizeof(dev_info->default_txconf));
>   
> -	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
> -	       sizeof(dev_info->rx_desc_lim));
> -	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
> -	       sizeof(dev_info->tx_desc_lim));
> +	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
> +	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
>   
>   	/**
>   	 * If dedicated hw queues enabled for link bonding device in LACP mode
> 

[dpdk-dev] [PATCH v3] net/bonding: fix create bonded device test failure

[Hari Kumar Vemula]

test_create_bonded_device is failing due to improper initialisation in
bonded device configuration. Which leads to crash while setting up queues.

The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
bonded device which fails.
This is due to "rx_desc_lim" is set to 0 as default value of bonded device
during bond_alloc().
Hence nb_rx_desc (1024) is > 0 and test fails.

Fix is to set the default values of rx_desc_lim of bonded device to
appropriate value.
Receive the values from slaves configuration like done for other existing
slave configuration

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
Acked-by: Chas Williams <chas3@att.com>
---
v3: Modified commit message
v2: Bonded device desc_lim values are received from slave configuration
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 44deaf119..23cec2549 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2228,6 +2228,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	uint16_t max_nb_rx_queues = UINT16_MAX;
 	uint16_t max_nb_tx_queues = UINT16_MAX;
+	uint16_t max_rx_desc_lim = UINT16_MAX;
+	uint16_t max_tx_desc_lim = UINT16_MAX;
 
 	dev_info->max_mac_addrs = BOND_MAX_MAC_ADDRS;
 
@@ -2252,6 +2254,12 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 			if (slave_info.max_tx_queues < max_nb_tx_queues)
 				max_nb_tx_queues = slave_info.max_tx_queues;
+
+			if (slave_info.rx_desc_lim.nb_max < max_rx_desc_lim)
+				max_rx_desc_lim = slave_info.rx_desc_lim.nb_max;
+
+			if (slave_info.tx_desc_lim.nb_max < max_tx_desc_lim)
+				max_tx_desc_lim = slave_info.tx_desc_lim.nb_max;
 		}
 	}
 
@@ -2263,10 +2271,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	memcpy(&dev_info->default_txconf, &internals->default_txconf,
 	       sizeof(dev_info->default_txconf));
 
-	memcpy(&dev_info->rx_desc_lim, &internals->rx_desc_lim,
-	       sizeof(dev_info->rx_desc_lim));
-	memcpy(&dev_info->tx_desc_lim, &internals->tx_desc_lim,
-	       sizeof(dev_info->tx_desc_lim));
+	dev_info->rx_desc_lim.nb_max = max_rx_desc_lim;
+	dev_info->tx_desc_lim.nb_max = max_tx_desc_lim;
 
 	/**
 	 * If dedicated hw queues enabled for link bonding device in LACP mode
-- 
2.17.2

Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net/bonding: fix create bonded device test failure

[Ferruh Yigit]

On 2/5/2019 1:39 PM, Hari Kumar Vemula wrote:
> test_create_bonded_device is failing due to improper initialisation in
> bonded device configuration. Which leads to crash while setting up queues.
> 
> The value of nb_rx_desc is checked if it is not in range of rx_desc_lim of
> bonded device which fails.
> This is due to "rx_desc_lim" is set to 0 as default value of bonded device
> during bond_alloc().
> Hence nb_rx_desc (1024) is > 0 and test fails.
> 
> Fix is to set the default values of rx_desc_lim of bonded device to
> appropriate value.
> Receive the values from slaves configuration like done for other existing
> slave configuration
> 
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> Acked-by: Chas Williams <chas3@att.com>

Applied to dpdk-next-net/master, thanks.