> On Jun 22, 2020, at 10:26 AM, Thomas Monjalon <tho...@monjalon.net> wrote: > > 19/06/2020 18:13, Dharmik Thakkar: >>> On Jun 19, 2020, at 10:43 AM, Thomas Monjalon <tho...@monjalon.net> wrote: >>> 19/06/2020 17:38, Dharmik Thakkar: >>>>> On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <tho...@monjalon.net> wrote: >>>>> 17/06/2020 20:21, Ferruh Yigit: >>>>>> On 5/20/2020 4:20 AM, Dharmik Thakkar 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.thak...@arm.com> >>>>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> >>>>>>> Reviewed-by: Phil Yang <phil.y...@arm.com> >>>>>>> --- >>>>>>> v3: >>>>>>> - Replace config/common_base with build/.config (Thomas) >>>>>>> >>>>>>> v2: >>>>>>> - Remove extra '#'. >>>>>>> --- >>>>>>> doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++ >>>>>>> 1 file changed, 12 insertions(+) >>>>>>> >>>>>>> diff --git a/doc/guides/testpmd_app_ug/build_app.rst >>>>>>> b/doc/guides/testpmd_app_ug/build_app.rst >>>>>>> index d1ca9f3d19a9..8c9aaa83187f 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,' >>>>>>> build/.config >>>>>>> + sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' >>>>>>> build/.config >>>>>>> + >>>>>>> + 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 >>>>>>> >>>>>> >>>>>> Hi Dharmik, >>>>>> >>>>>> This patch waiting to address the Thomas comment on document how to >>>>>> enable those >>>>>> config options in meson. @Thomas, can you please correct if I got it >>>>>> wrong? >>>>> >>>>> If the doc is only about make, it will be removed in 2 months. >>>>> So yes, I think it doesn't make sense to be merged as is. >>>>> >>>> >>>> Make sense. >>>> >>>> With meson, I see multiple ways to enable configuration options: >>>> >>>> 1] macro in $(RTE_SDK)/config/rte_config.h >>>> 2] using dpdk_conf.set() in $(RTE_SDK)/config/meson.build >>>> >>>> I’d appreciate some suggestions on the best way to enable these options. >>> >>> Neither 1 nor 2 because both are source files. >>> Configuration must be done in the build directory. >>> >>> We use "meson -D" to configure options listed in meson_options.txt >>> The real question is which option we accept to be added. >>> >>> >> >> Yes, correct. There are suggestions for making these as runtime >> configuration functions which can be enabled(or disabled) using ’set’ from >> within the application. > > I don't understand this proposal. > Is there a patch? > >> Do you think, till this change is implemented, these options can be included >> in meson_options.txt? > > In general, if you think there is a better alternative, > please propose a patch.
I am not sure if changing it to a runtime function is a better alternative. I am expecting some performance degradation with the change since the macros will be replaced with ‘if’ statements. I will hack up a patch to see the performance difference and propose if there is not much of a difference. > > I have no opinion what is best to manage such option.
