Re: [dpdk-dev] [PATCH v5 03/20] doc: remove references to make from Linux guide

[Thomas Monjalon <thomas@monjalon.net>]

Hi,

Below are some comments. If you agree, I will do the changes
myself while pushing to the main branch.


21/09/2020 15:59, Ciara Power:
> Make is no longer supported for compiling DPDK, references are now
> removed in the documentation.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  doc/guides/linux_gsg/build_dpdk.rst        | 16 +++++++++++-----
>  doc/guides/linux_gsg/build_sample_apps.rst | 18 +++---------------
>  doc/guides/linux_gsg/enable_func.rst       | 13 +++++--------
>  doc/guides/linux_gsg/linux_drivers.rst     | 18 +++++++++++-------
>  doc/guides/linux_gsg/sys_reqs.rst          |  6 +-----
>  5 files changed, 31 insertions(+), 40 deletions(-)
> 
> diff --git a/doc/guides/linux_gsg/build_dpdk.rst b/doc/guides/linux_gsg/build_dpdk.rst
> index a895e1b0f4..85d04520bf 100644
> --- a/doc/guides/linux_gsg/build_dpdk.rst
> +++ b/doc/guides/linux_gsg/build_dpdk.rst
> @@ -76,6 +76,8 @@ and the last step causing the dynamic loader `ld.so` to update its cache to take
>     distributions, `/usr/local/lib` and `/usr/local/lib64` should be added
>     to a file in `/etc/ld.so.conf.d/` before running `ldconfig`.
>  
> +.. _adjusting_build_options:
> +

extra blank line

>  
>  Adjusting Build Options
>  ~~~~~~~~~~~~~~~~~~~~~~~
> @@ -112,6 +114,9 @@ dependencies are met on the current system are built.
>  When `-Dexamples=all` is set as a meson option, meson will check each example application to see if it can be built,
>  and add all which can be built to the list of tasks in the ninja build configuration file.
>  
> +.. _building_app_using_installed_dpdk:
> +
> +

extra blank line

>  Building Applications Using Installed DPDK
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> @@ -135,11 +140,12 @@ and the sources for that build are stored in ``$(SRCS-y)``.
>  
>  .. note::
>  
> -   Unlike with the older make build system, the meson system is not
> -   designed to be used directly from a build directory. Instead it is
> -   recommended that it be installed either system-wide or to a known
> -   location in the user's home directory. The install location can be set
> -   using the `--prefix` meson option (default: `/usr/local`).
> +   Unlike with the make build system present used in older DPDK releases,
> +   the meson system is not designed to be used directly from a build
> +   directory. Instead it is recommended that it be installed either
> +   system-wide or to a known location in the user's home directory.
> +   The install location can be set using the `--prefix` meson option
> +   (default: `/usr/local`).

I prefer the first version.
The words "system present used" look weird.
Anyway, if any reword is needed, better to change only the impacted
line and keep the rest of the line wrapping untouched.

>  
>  an equivalent build recipe for a simple DPDK application using meson as a
>  build system is shown below:
> diff --git a/doc/guides/linux_gsg/build_sample_apps.rst b/doc/guides/linux_gsg/build_sample_apps.rst
> index 2882883f95..f4bf0e71e5 100644
> --- a/doc/guides/linux_gsg/build_sample_apps.rst
> +++ b/doc/guides/linux_gsg/build_sample_apps.rst
> @@ -1,7 +1,7 @@
>  ..  SPDX-License-Identifier: BSD-3-Clause
>      Copyright(c) 2010-2014 Intel Corporation.
>  
> -Compiling and Running Sample Applications
> +Running Sample Applications
>  =========================================

Underlining must be adjusted

>  
>  The chapter describes how to compile and run applications in an DPDK environment.
> @@ -119,7 +119,7 @@ Copy the DPDK application binary to your target, then run the application as fol
>  (assuming the platform has four memory channels per processor socket,
>  and that cores 0-3 are present and are to be used for running the application)::
>  
> -    ./helloworld -l 0-3 -n 4
> +    ./dpdk-helloworld -l 0-3 -n 4
>  
>  .. note::
>  
> @@ -183,19 +183,7 @@ If the DPDK cannot allocate enough memory on each socket, the EAL initialization
>  Additional Sample Applications
>  ------------------------------
>  
> -Additional sample applications are included in the ${RTE_SDK}/examples directory.
> +Additional sample applications are included in the DPDK examples directory.
>  These sample applications may be built and run in a manner similar to that described in earlier sections in this manual.
>  In addition, see the *DPDK Sample Applications User Guide* for a description of the application,
>  specific instructions on compilation and execution and some explanation of the code.
> -
> -Additional Test Applications
> -----------------------------
> -
> -In addition, there are two other applications that are built when the libraries are created.
> -The source files for these are in the DPDK/app directory and are called test and testpmd.
> -Once the libraries are created, they can be found in the build/app directory.
> -
> -*   The test application provides a variety of specific tests for the various functions in the DPDK.
> -
> -*   The testpmd application provides a number of different packet throughput tests and
> -    examples of features such as how to use the Flow Director found in the Intel® 82599 10 Gigabit Ethernet Controller.

OK to remove the test applications from the page dedicated to samples.
A lot of similar cleanup could be done if someone volunteers.

> --- a/doc/guides/linux_gsg/enable_func.rst
> +++ b/doc/guides/linux_gsg/enable_func.rst
[...]
> @@ -128,13 +130,8 @@ Loading the DPDK KNI Kernel Module
>  ----------------------------------
>  
>  To run the DPDK Kernel NIC Interface (KNI) sample application, an extra kernel module (the kni module) must be loaded into the running kernel.
> -The module is found in the kmod sub-directory of the DPDK target directory.
> -Similar to the loading of the ``igb_uio`` module, this module should be loaded using the insmod command as shown below
> -(assuming that the current directory is the DPDK target directory):
> -
> -.. code-block:: console
> -
> -   insmod kmod/rte_kni.ko
> +The module is found in the kernel/linux sub-directory of the DPDK build directory.
> +This can be enabled in the same way as the ``igb_uio`` module, please see :ref:`load_uio` for details.

The only relevant info in "load_uio" chapter is to use insmod.
I think it is better to keep some lines above:

It should be loaded using the insmod command::

   insmod kmod/rte_kni.ko

> --- a/doc/guides/linux_gsg/linux_drivers.rst
> +++ b/doc/guides/linux_gsg/linux_drivers.rst
> @@ -12,6 +12,9 @@ Different PMDs may require different kernel drivers in order to work properly.
>  Depends on the PMD being used, a corresponding kernel driver should be load
>  and bind to the network ports.
>  
> +.. _load_uio:
> +
> +

This anchor is not need in my opinion.

>  UIO
>  ---
>  
> @@ -28,19 +31,20 @@ can provide the uio capability. This module can be loaded using the command:
>      ``uio_pci_generic`` module doesn't support the creation of virtual functions.
>  
>  As an alternative to the ``uio_pci_generic``, the DPDK also includes the igb_uio
> -module which can be found in the kmod subdirectory referred to above. It can
> +module which can be found in the kernel/linux subdirectory referred to above. It can
>  be loaded as shown below:
>  
>  .. code-block:: console
>  
>      sudo modprobe uio
> -    sudo insmod kmod/igb_uio.ko
> +    sudo insmod <build_dir>/kernel/linux/igb_uio/igb_uio.ko
>  
>  .. note::
>  
> -   ``igb_uio`` module is disabled by default starting from ``DPDK v20.02``.
> -   To build it, the config option ``CONFIG_RTE_EAL_IGB_UIO`` should be enabled.
> -   It is planned to move ``igb_uio`` module to a different git repository.
> +   Building DPDK Linux kernel modules is disabled by default starting from DPDK v20.02.
> +   To enable them again, the config option "enable_kmods" needs to be set in the meson
> +   build configuration. See :ref:`adjusting_build_options` for details on how to set/clear
> +   build options. It is planned to move ``igb_uio`` module to a different git repository.

In general it is better to split the lines logically to ease future patches diff.