[dpdk-dev] [PATCH v3 22/37] doc: remove references to make in contributing guides

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Monday, September 21, 2020 5:15 PM
> To: Power, Ciara <ciara.power at intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu at intel.com>
> Cc: dev at dpdk.org; Mcnamara, John <john.mcnamara at intel.com>;
> Kovacevic, Marko <marko.kovacevic at intel.com>; Richardson, Bruce
> <bruce.richardson at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 22/37] doc: remove references to make in
> contributing guides
> 
> 17/09/2020 15:44, Power, Ciara:
> > Hi Thomas,
> >
> > One comment inline.
> >
> > Thanks,
> > Ciara
> >
> >
> > >-----Original Message-----
> > >From: Thomas Monjalon <thomas at monjalon.net>
> > >Sent: Monday 7 September 2020 11:21
> > >To: Power, Ciara <ciara.power at intel.com>; dev at dpdk.org
> > >Cc: Kilheeney, Louise <louise.kilheeney at intel.com>; Mcnamara, John
> > ><john.mcnamara at intel.com>; Kovacevic, Marko
> > ><marko.kovacevic at intel.com>
> > >Subject: Re: [dpdk-dev] [PATCH v3 22/37] doc: remove references to
> make in
> > >contributing guides
> > >
> >
> > <snip>
> >
> > >[...]
> > >> -Library Statistics
> > >> -------------------
> > >> -
> > >> -Description
> > >> -~~~~~~~~~~~
> > >> -
> > >> -This document describes the guidelines for DPDK library-level
> > >> statistics counter -support. This includes guidelines for turning
> > >> library statistics on and off and -requirements for preventing ABI
> > >> changes when implementing statistics.
> > >
> > >I think the ABI part is still relevant.
> >
> > Looking for some clarification on this. Which parts (if any) of the library
> statistics section should be
> > added back in for the ABI section below to make sense being included? Or
> what do you suggest doing here?
> 
> I think we can consider we still have the ability to turn stats off
> because of the #ifdef still being there.
> Cristian, do you think we should remove this doc?
> Is on/off still possible in the new pipeline libs?
> 

Hi Ciara and Thomas,

IMO this section is still relevant and it is unrelated to the topic of this patch set, why do we want to remove it?

>From the make removal point of view, we only need the following change, and everything else is still logically valid:
-   # DPDK file config/common_linux, config/common_freebsd, etc.
-   CONFIG_RTE_<LIBRARY_NAME>_STATS_COLLECT=y/n
+   /* Library API header file. */
+  #define RTE_<LIBRARY_NAME>_STATS_COLLECT

On the larger stats topic, I think the section is still relevant, especially from the point of view of avoiding ABI breakage due to stats, which is applicable to several libraries, so I suggest to keep it.

In the new pipeline libs (rte_swx_pipeline.h API), all the stats are enabled by default.

Regards,
Cristian

> 
> > >[...]
> > >> -Prevention of ABI changes due to library statistics support
> > >> -
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~
> > >> -
> > >> -The layout of data structures and prototype of functions that are
> > >> part of the -library API should not be affected by whether the
> > >> collection of statistics -counters is turned on or off for the current
> > >> library. In practical terms, this -means that space should always be
> > >> allocated in the API data structures for -statistics counters and the
> > >> statistics related API functions are always built -into the code,
> > >> regardless of whether the statistics counter collection is turned -on
> > >> or off for the current library.
> > >> -
> > >> -When the collection of statistics counters for the current library is
> > >> turned -off, the counters retrieved through the statistics related API
> > >> functions should -have a default value of zero.
> 
>