Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Thomas Monjalon <thomas@monjalon.net>]

19/06/2020 18:13, Dharmik Thakkar:
> > On Jun 19, 2020, at 10:43 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 19/06/2020 17:38, Dharmik Thakkar:
> >>> On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <thomas@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.thakkar@arm.com>
> >>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>>>> Reviewed-by: Phil Yang <phil.yang@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 have no opinion what is best to manage such option.