Re: [dpdk-dev] [PATCH v5] doc: introduce openwrt how-to guide

[Ye Xiaolong <xiaolong.ye@intel.com>]

Hi, Thomas

Thanks a lot for your detailed review.

On 02/16, Thomas Monjalon wrote:
>18/01/2020 06:48, Xiaolong Ye:
>> This doc describes how to enable DPDK on Openwrt in both virtual and
>> physical x86 environment.
>> 
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> ---
>> --- /dev/null
>> +++ b/doc/guides/howto/openwrt.rst
>> +Enable DPDK on openwrt
>> +======================
>> +
>> +This document describes how to enable Data Plane Development Kit (DPDK) on
>> +Openwrt in both a virtual and physical x86 environment.
>
>Please take care of the uppercases in OpenWrt.

Got it, will stick to OpenWrt for all occurrences.

>
>> +
>> +Introduction
>> +------------
>> +
>> +The OpenWrt project is a Linux operating system targeting embedded devices.
>> +Instead of trying to create a single, static firmware, OpenWrt provides a fully
>> +writable filesystem with package management. This frees the user from the
>> +application selection and configuration provided by the vendor and allows users
>> +to customize the device through the use of packages to suit any application. For
>> +developers OpenWrt is the framework to build an application without having to
>> +build a complete firmware around it. For users is offers full customization
>> +to use the device in ways never envisioned.
>
>Why doing marketing for OpenWrt in DPDK doc?
>It is a copy paste from https://openwrt.org/
>I think it is enough to say OpenWrt is a source-based router OS with package management.

Just want to gave some basic intro about OpenWrt, but it seems too much.
Will adopt your suggestion in next version.

>
>> +
>> +Pre-requisites
>> +~~~~~~~~~~~~~~
>> +
>> +You need gcc, binutils, bzip2, flex, python3.5+, perl, make, find, grep, diff,
>> +unzip, gawk, getopt, subversion, libz-dev and libc headers installed.
>
>These are prerequisites for building, so should be in section below.
>Maybe we can just provide a link to the official doc instead.
>

Make sense, will provide the link in next version.

>> +
>> +Build OpenWrt
>> +-------------
>> +
>> +You can obtain OpenWrt image through https://downloads.openwrt.org/releases. To
>
>Please take the habit of splitting lines after a punctuation.
>

Will keep this in mind.

>> +fully customize your own OpenWrt, it is highly recommended to build it from
>> +the source code. You can clone the OpenWrt source code as follows:
>> +
>> +.. code-block:: console
>> +
>> +    git clone https://git.openwrt.org/openwrt/openwrt.git
>> +
>> +OpenWrt configuration
>> +~~~~~~~~~~~~~~~~~~~~~
>> +
>> +* Select ``x86`` in ``Target System``
>> +* Select ``x86_64`` in ``Subtarget``
>> +* Select ``Build the OpenWrt SDK`` for cross-compilation environment
>> +* Select ``Use glibc`` in ``Advanced configuration options (for developers)``
>> +  then ``ToolChain Options`` and ``C Library implementation``
>> +
>> +Kernel configuration
>> +~~~~~~~~~~~~~~~~~~~~
>> +
>> +The following configurations should be enabled:
>> +
>> +* ``CONFIG_UIO=y``
>
>No, UIO is not recommended anymore.

Got it, will recommend vfio instead.

>
>> +* ``CONFIG_HUGETLBFS=y``
>> +* ``CONFIG_HUGETLB_PAGE=y``
>> +* ``CONFIG_PAGE_MONITOR=y``
>
>Why PAGE_MONITOR?

Good catch, should be CONFIG_PROC_PAGE_MONITOR.

>
>> +
>> +Build steps
>> +~~~~~~~~~~~
>> +
>> +For detailed OpenWrt build steps, please refer to the
>> +`OpenWrt build guide
>> +<https://openwrt.org/docs/guide-developer/build-system/use-buildsystem>`_.
>> +
>> +After the build is completed, you can find the images and sdk in
>> +``<OpenWrt Root>/bin/targets/x86/64-glibc/``.
>
>s/sdk/SDK/

Got it.

>
>[...]
>> +    [binaries]
>> +    c = 'x86_64-openwrt-linux-gcc'
>> +    cpp = 'x86_64-openwrt-linux-cpp'
>> +    ar = 'x86_64-openwrt-linux-ar'
>> +    strip = 'x86_64-openwrt-linux-strip'
>> +
>> +    meson builddir --cross-file openwrt-cross
>
>This is really a good example why meson should allow to override the toolchain prefix.

Not sure whether I get your point or not, do you mean meson should support
something like "cross = 'x86_64-openwrt-linux-'" in config file (just like the
CROSS_COMPILE for make build system),  so we don't need to override the build
tools one by one?

>Please work with meson on this topic.

Sure, I guess time is not enough to do it in this release, will plan for
next one.

>
>[...]
>> +To cross compile with make:
>> +
>> +.. code-block:: console
>> +
>> +    export STAGING_DIR=<OpenWrt sdk>/glibc/openwrt-sdk-x86-64_gcc-8.3.0_glibc.Linux-x86_64/staging_dir
>> +    export RTE_SDK=`pwd`
>> +    export RTE_KERNELDIR=<OpenWrt Root>/build_dir/target-x86_64_glibc/linux-x86_64/linux-4.19.81/
>> +    make config T=x86_64-native-linuxapp-gcc
>> +    make -j 100 CROSS=x86_64-openwrt-linux-gnu-
>
>The make build system should not be documented as it is going to be deprecated.

Got it.

>
>> +
>> +Running DPDK application on OpenWrt
>> +-----------------------------------
>> +
>> +Virtual machine
>> +~~~~~~~~~~~~~~~
>> +
>> +* Extract the boot image
>> +
>> +.. code-block:: console
>> +
>> +    gzip -d openwrt-x86-64-combined-ext4.img.gz
>> +
>> +* Launch Qemu
>> +
>> +.. code-block:: console
>> +
>> +    qemu-system-x86_64 \
>> +            -cpu host \
>> +            -smp 8 \
>> +            -enable-kvm \
>> +            -M q35 \
>> +            -m 2048M \
>> +            -object memory-backend-file,id=mem,size=2048M,mem-path=/tmp/hugepages,share=on \
>> +            -drive file=<Your OpenWrt images folder>/openwrt-x86-64-combined-ext4.img,id=d0,if=none,bus=0,unit=0 \
>> +            -device ide-hd,drive=d0,bus=ide.0 \
>> +            -net nic,vlan=0 \
>> +            -net nic,vlan=1 \
>> +            -net user,vlan=1 \
>> +            -display none \
>> +
>> +
>> +Physical machine
>> +~~~~~~~~~~~~~~~~
>> +
>> +If you are using a Windows PC, you can use an image writer application such as
>> +``Win32 Disk Imager`` and ``Etcher`` to write the OpenWrt image
>> +(openwrt-x86-64-combined-ext4.img) to a USB flash driver or SDcard.
>
>If you just built a DPDK for Linux, you are not running Windows,
>except if you are using a Linux server for compilation.
>Anyway, we should not try to document such procedure in my opinion.
>I'm sure you can refer to the OpenWrt documentation for flashing the image.

Ok, I'll stay focus on Linux for this doc.

>
>> +
>> +If you are using Linux, you can use the ``dd`` tool to write the OpenWrt image
>> +to the drive you want to write the image on.
>> +
>> +.. code-block:: console
>> +
>> +    dd if=openwrt-18.06.1-x86-64-combined-squashfs.img of=/dev/sdX
>> +
>> +Where sdX is name of the drive. (You can find it though ``fdisk -l``)
>> +
>> +Running DPDK
>> +~~~~~~~~~~~~
>> +
>> +More detailed info about how to run a DPDK application please refer to
>> +``Running DPDK Applications`` section of :ref:`the DPDK documentation <linux_gsg>`.
>> +
>> +.. note::
>> +
>> +    You need to install pre-built numa libraries (including soft link)
>
>s/numa/NUMA/

Got it.

>
>> +    to /usr/lib64 in OpenWrt.
>
>libnuma is not packaged in OpenWrt?
>

Unfortunately not, I guess the reason maybe OpenWrt is mainly focused on low-end
embedded systems which don't have numa support. 

>
>> --- a/doc/guides/rel_notes/release_20_02.rst
>> +++ b/doc/guides/rel_notes/release_20_02.rst
>> @@ -116,6 +116,10 @@ New Features
>>  
>>    Added support for ESP rte_flow patterns to the testpmd application.
>>  
>> +* **Added OpenWrt howto guide.**
>> +
>> +  Added document describes how to enable DPDK on OpenWrt in both virtual and
>> +  physical machine.
>>  
>>  Removed Items
>>  -------------
>
>You miss a doubled blank line before this title.

Will keep 2 blank lines before "Removed Items".

Thanks,
Xiaolong