Hi Thomas,
Thanks for reviewing, these changes seem fine overall, comments inline.
Thanks,
Ciara
>-----Original Message-----
>From: Thomas Monjalon <tho...@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.
