diff mbox series

[v2,4/5] doc: include config options in testpmd user guide

Message ID 20200508223829.3228-4-dharmik.thakkar@arm.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers
Series [v2,1/5] app/testpmd: print clock with CPU cycles per pkt |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Dharmik Thakkar May 8, 2020, 10:38 p.m. UTC 
  Update testpmd documentation to include RECORD configuration options,
CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v2:
 - Remove extra '#'.
---
 doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Iremonger, Bernard May 12, 2020, 10:20 a.m. UTC | #1 
> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Friday, May 8, 2020 11:38 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH v2 4/5] doc: include config options in testpmd user guide
> 
> Update testpmd documentation to include RECORD configuration options,
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
Thomas Monjalon May 19, 2020, 7:42 a.m. UTC | #2 
> > Update testpmd documentation to include RECORD configuration options,
> > CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> > CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> > 
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> 
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

How these options are managed with meson?
Thomas Monjalon May 19, 2020, 7:45 a.m. UTC | #3 
09/05/2020 00:38, Dharmik Thakkar:
> Update testpmd documentation to include RECORD configuration options,
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
> +#.  If required, enable configuration options. For example:
> +
> +    .. code-block:: console
> +
> +        cd to the top-level DPDK directory
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' config/common_base
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' config/common_base

Changing source code is wrong.
Only the generated .config file should be changed.

Not even speaking about meson...

Why we don't have any review on documentation patches?

Patch dropped from next-net pulling.
Dharmik Thakkar May 19, 2020, 10:58 p.m. UTC | #4 
> On May 19, 2020, at 2:42 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
>>> Update testpmd documentation to include RECORD configuration options,
>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>> 
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>> 
>> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> How these options are managed with meson?
> 
> 

As per my understanding, currently, this is not implemented with meson.
With ‘make’, the configuration options are saved within ./build/include/rte_config.h ( which gets generated during make config …).
But this file (rte_config.h) does not get generated when using meson.
Thomas Monjalon May 20, 2020, 7:53 a.m. UTC | #5 
20/05/2020 00:58, Dharmik Thakkar:
> > On May 19, 2020, at 2:42 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> >>> Update testpmd documentation to include RECORD configuration options,
> >>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> >>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> >>> 
> >>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >> 
> >> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > 
> > How these options are managed with meson?
> 
> As per my understanding, currently, this is not implemented with meson.
> With ‘make’, the configuration options are saved within ./build/include/rte_config.h ( which gets generated during make config …).
> But this file (rte_config.h) does not get generated when using meson.

That's also my understanding.
There is a gap which needs to be fixed, not sure what is the best approach.
Can it be made a runtime option?
Dharmik Thakkar May 20, 2020, 10:39 p.m. UTC | #6 
> On May 20, 2020, at 2:53 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 20/05/2020 00:58, Dharmik Thakkar:
>>> On May 19, 2020, at 2:42 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>> Update testpmd documentation to include RECORD configuration options,
>>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>>>> 
>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>> 
>>>> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
>>> 
>>> How these options are managed with meson?
>> 
>> As per my understanding, currently, this is not implemented with meson.
>> With ‘make’, the configuration options are saved within ./build/include/rte_config.h ( which gets generated during make config …).
>> But this file (rte_config.h) does not get generated when using meson.
> 
> That's also my understanding.
> There is a gap which needs to be fixed, not sure what is the best approach.

IMO, for now, it is best to replicate what is being done for ‘make’.

> Can it be made a runtime option?

Yes, it can be made a runtime option but that will be a separate patch.

>
diff mbox series

Patch

diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
index d1ca9f3d19a9..b93c82b8c115 100644
--- a/doc/guides/testpmd_app_ug/build_app.rst
+++ b/doc/guides/testpmd_app_ug/build_app.rst
@@ -21,6 +21,18 @@  The basic compilation steps are:
 
         export RTE_TARGET=x86_64-native-linux-gcc
 
+#.  If required, enable configuration options. For example:
+
+    .. code-block:: console
+
+        cd to the top-level DPDK directory
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' config/common_base
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' config/common_base
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
+
 #.  Build the application:
 
     .. code-block:: console