Re: [dpdk-dev] [PATCH v7 03/14] doc: remove references to make from NICs guides

["Power, Ciara" <ciara.power@intel.com>]

Hi Thomas,

Thanks for reviewing, these changes seem fine overall, comments inline.

Thanks,
Ciara

>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Thursday 22 October 2020 09:32
<snip>
>Subject: Re: [dpdk-dev] [PATCH v7 03/14] doc: remove references to make
>from NICs guides
>
>Hi,
>
>I would like to apply this series.
>I see some small things that I could fix.
>Please see the comments below to confirm.
>
>21/10/2020 10:17, Ciara Power:
>> -.. _bnx2x_driver-compilation:
>> + .. _bnx2x_driver-compilation:
>
>was it changed by mistake?
>

Yes sorry, that change was unintentional.

>[...]
>>  #. Load ``igb_uio`` or ``vfio-pci`` driver:
>>
>> +   Before compiling, make sure to enable kmods for the meson build::
>> +
>> +      meson configure -Denable_kmods=true
>> +
>
>igb_uio is moved so I think this addition is not relevant anymore.
>

Yes agreed, can remove.

>[...]
>> -- ``CONFIG_RTE_LIBRTE_DPAA_DEBUG_DRIVER`` (default ``n``)
>> -
>> -  Toggles display of bus configurations and enables a debugging queue
>> -  to fetch error (Rx/Tx) packets to driver. By default, packets with
>> errors
>> -  (like wrong checksum) are dropped by the hardware.
>> -
>> -- ``CONFIG_RTE_LIBRTE_DPAA_HWDEBUG`` (default ``n``)
>> -
>> -  Enables debugging of the Queue and Buffer Manager layer which
>> interacts
>> -  with the DPAA hardware.
>
>I feel these explanations should be kept.
>Or do you think it's not worth?
>

Yes sure, keep them.

>[...]
>> -- ``CONFIG_RTE_LIBRTE_DPAA2_DEBUG_DRIVER`` (default ``n``)
>> -
>> -  Toggle display of debugging messages/logic
>> -
>> -- ``CONFIG_RTE_LIBRTE_DPAA2_USE_PHYS_IOVA`` (default ``n``)
>> -
>> -  Toggle to use physical address vs virtual address for hardware
>accelerators.
>
>Keep these ones?
>

Yes can keep, although one thing to note is I think RTE_LIBRTE_DPAA2_USE_PHYS_IOVA is now enabled by default, going by config/meson.build:
	dpdk_conf.set('RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', true)

>[...]
>> -   * **CONFIG_RTE_LIBRTE_ENA_DEBUG_RX** (default n): Enables or
>disables debug
>> -     logging of RX logic within the ENA PMD driver.
>> -
>> -   * **CONFIG_RTE_LIBRTE_ENA_DEBUG_TX** (default n): Enables or
>disables debug
>> -     logging of TX logic within the ENA PMD driver.
>> -
>> -   * **CONFIG_RTE_LIBRTE_ENA_COM_DEBUG** (default n): Enables or
>disables debug
>> -     logging of low level tx/rx logic in ena_com(base) within the ENA PMD
>driver.
>
>Keep? Or debug options not worth?
>

Yes sure, I have no problems keeping them.

>[...]
>> -- ``CONFIG_RTE_IBVERBS_LINK_DLOPEN`` (default **n**)
>> -
>> -  Build PMD with additional code to make it loadable without hard
>> -  dependencies on **libibverbs** nor **libmlx5**, which may not be
>> installed
>> -  on the target system.
>> -
>> -  In this mode, their presence is still required for it to run
>> properly,
>> -  however their absence won't prevent a DPDK application from
>> starting (with
>> -  ``CONFIG_RTE_BUILD_SHARED_LIB`` disabled) and they won't show up
>as
>> -  missing with ``ldd(1)``.
>> -
>> -  It works by moving these dependencies to a purpose-built rdma-core
>"glue"
>> -  plug-in which must either be installed in a directory whose name is
>> based
>> -  on ``CONFIG_RTE_EAL_PMD_PATH`` suffixed with ``-glue`` if set, or
>> in a
>> -  standard location for the dynamic linker (e.g. ``/lib``) if left to
>> the
>> -  default empty string (``""``).
>> -
>> -  This option has no performance impact.
>> -
>> -- ``CONFIG_RTE_IBVERBS_LINK_STATIC`` (default **n**)
>> -
>> -  Embed static flavor of the dependencies **libibverbs** and
>> **libmlx5**
>> -  in the PMD shared library or the executable static binary.
>> -
>> -- ``CONFIG_RTE_LIBRTE_MLX5_DEBUG`` (default **n**)
>> -
>> -  Toggle debugging code and stricter compilation flags. Enabling this
>> option
>> -  adds additional run-time checks and debugging messages at the cost
>> of
>> -  lower performance.
>> -
>> -.. note::
>> -
>> -   For BlueField, target should be set to ``arm64-bluefield-linux-gcc``. This
>> -   will enable ``CONFIG_RTE_LIBRTE_MLX5_PMD`` and set
>``RTE_CACHE_LINE_SIZE`` to
>> -   64. Default armv8a configuration of make build and meson build set it to
>128
>> -   then brings performance degradation.
>> -
>> -This option is available in meson:
>> +The ibverbs libraries can be linked with this PMD in a number of
>> +ways, configured by the "ibverbs_link" build option. This can take on
>> +the following values:
>>
>>  - ``ibverbs_link`` can be ``static``, ``shared``, or ``dlopen``.
>
>I would reword to include explanations above.
>

I agree, I made that change for the vdpadevs/mlx5 doc (https://patchwork.dpdk.org/patch/81678/) , I missed it in these NIC guides.

>[...]
>> -- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX`` (default **n**)
>> -
>> -  Toggle display of transmit fast path run-time messages.
>> -
>> -- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_RX`` (default **n**)
>> -
>> -  Toggle display of receive fast path run-time messages.
>> -
>> -- ``CONFIG_RTE_LIBRTE_QEDE_FW`` (default **""**)
>> +- ``RTE_LIBRTE_QEDE_FW`` (default **""**)
>>
>>    Gives absolute path of firmware file.
>>    ``Eg: "/lib/firmware/qed/qed_init_values-8.40.33.0.bin"``
>> @@ -130,6 +117,16 @@ enabling debugging options may affect system
>performance.
>>    CAUTION this option is more for custom firmware, it is not
>>    recommended for use under normal condition.
>>
>> +The following options can be enabled with Meson flags.
>> +
>> +- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX`` (default **disabled**)
>> +
>> +  Toggle display of transmit fast path run-time messages.
>> +
>> +- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_RX`` (default **disabled**)
>> +
>> +  Toggle display of receive fast path run-time messages.
>
>CONFIG_ should be removed.
>

 Agreed, missed that when adding them back in, thanks.