Re: [dpdk-dev] [PATCH v2 1/8] ether: refine debug compile option

[Thomas Monjalon <thomas@monjalon.net>]

16/03/2021 14:05, Ferruh Yigit:
> On 3/12/2021 12:12 PM, Qi Zhang wrote:
> > PMDs use RTE_LIBRTE_<PMD_NAME>_DEBUG_RX|TX as compile option to wrap
> > data path debug code. As .config has been removed since the meson build,
> > It is not friendly for new DPDK users to notice those debug options.
> > 
> > The patch introduces below compile options for specific Rx/Tx data path
> > debug, so PMD can choose to reuse them to avoid maintain their own.
> > 
> > - RTE_LIBRTE_ETHDEV_DEBUG_RX
> > - RTE_LIBRTE_ETHDEV_DEBUG_TX
> > 
> > Also, all the compile options are documented on the overview page, so
> > users can easily find them.

English question: is "compile option" correct,
or should it be "compilation option"?
Cc Bruce to have a native in the discussion :)


> > --- a/doc/guides/nics/overview.rst
> > +++ b/doc/guides/nics/overview.rst
> > @@ -32,3 +32,23 @@ More details about features can be found in :doc:`features`.
> > +The ethdev layer support below compile options for debug purpose:

s/support/supports/

> > +
> > +- ``RTE_LIBRTE_ETHDEV_DEBUG`` (default **disabled**)
> > +
> > +  Compile with debug code on data path.

What is data path if not Rx or Tx?

> > +
> > +- ``RTE_LIBRTE_ETHDEV_DEBUG_RX`` (default **disabled**)
> > +
> > +  Compile with debug code on Rx data path.
> > +
> > +- ``RTE_LIBRTE_ETHDEV_DEBUG_TX`` (default **disabled**)
> > +
> > +  Compile with debug code on Tx data path.

In general, I think "LIBRTE_" is redundant and useless as macro prefix.

> > +
> > +.. Note::
> > +
> > +   The lib_ethdev use above options to wrap debug code to trace invalid parameters on

s/lib_ethdev/ethdev library/

> > +   data path APIs, so performance downgrade is expected when enable those options.

s/enable/enabling/

> > +   Each PMD can decide to reuse them to wrap their own debug code in the Rx/Tx path.

Oh yes it could reduce the number of options.


> Overall patch looks good to me, but not sure about adding the documentation to 
> the NIC overview page. What do you think about moving the doc to next chapter, 
> under "3.1. Driver Compilation"?

+1