[PATCH] dts: improve documentation

Juraj Linkeš juraj.linkes at pantheon.tech
Fri Jan 12 14:24:56 CET 2024


I have two extra suggestions apart from the comments below:
There's a typo inside the "How To Write a Test Suite" section:
In that case, nothing will happen when they're is executed.

And Mypy is missing from the list of linters in the "DTS Developer
Tools" section, could you please add it?

> diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> index 32c18ee472..31495cad51 100644
> --- a/doc/guides/tools/dts.rst
> +++ b/doc/guides/tools/dts.rst
<snip>
> @@ -335,3 +336,183 @@ There are three tools used in DTS to help with code checking, style and formatti
>  These three tools are all used in ``devtools/dts-check-format.sh``,
>  the DTS code check and format script.
>  Refer to the script for usage: ``devtools/dts-check-format.sh -h``.
> +
> +Configuration Schema
> +--------------------

Just out of curiosity, is this generated from the schema? It's a
pretty neat format, but maintaining it could be a nightmare without a
script that would always produce the same format.

> +
> +Definitions
> +~~~~~~~~~~~
> +

The section names look to be taken from the schema and all of the
terminology is taken from the schema. Would it make sense to use YAML
terminology here, since people will try to use this information in
YAML files? Or maybe explain what we mean by definitions, properties,
objects, arrays or maybe some other things which YAML doesn't specify.

> +_`Node name`
> +   *string* – A unique identifier for a node. **Examples**: ``SUT1``, ``TG1``.
> +
> +_`ARCH`
> +   *string* – The CPU architecture. **Supported values**: ``x86_64``, ``arm64``, ``ppc64le``.
> +
> +_`CPU`
> +   *string* – The CPU microarchitecture. Use ``native`` for x86. **Supported values**: ``native``, ``armv8a``, ``dpaa2``, ``thunderx``, ``xgene1``.
> +
> +_`OS`
> +   *string* – The operating system. **Supported values**: ``linux``.
> +
> +_`Compiler`
> +   *string* – The compiler used for building DPDK. **Supported values**: ``gcc``, ``clang``, ``icc``, ``mscv``.
> +
> +_`Build target`
> +   *object* – Build targets supported by DTS for building DPDK, described as:
> +
> +   ==================== =========================================================================================
> +   ``arch``             See `ARCH`_
> +   ``os``               See `OS`_
> +   ``cpu``              See `CPU`_
> +   ``compiler``         See `Compiler`_
> +   ``compiler_wrapper`` *string* – Value prepended to the CC variable for the DPDK build.
> +
> +                        **Example**: ``ccache``
> +   ==================== =========================================================================================
> +
> +_`hugepages`
> +   *object* – hugepages described as:
> +
> +   ==================== ================================================================
> +   ``amount``           *integer* – The amount of hugepages to configure.
> +

We should update "amount" (uncountable) to something that's countable,
such as count or number.

> +                        Hugepage size will be the system default.
> +   ``force_first_numa`` (*optional*, defaults to ``false``) – If ``true``, it forces the
> +
> +                        configuration of hugepages on the first NUMA node.
> +   ==================== ================================================================
> +
> +_`Network port`
> +   *object* – the NIC port described as:
> +
> +   ====================== =================================================================================
> +   ``pci``                *string* – the local PCI address of the port. **Example**: ``0000:00:08.0``
> +   ``os_driver_for_dpdk`` | *string* – this port's device driver when using with DPDK
> +                          | When setting up the SUT, DTS will bind the network device to this driver
> +                          | for compatibility with DPDK.
> +
> +                          **Examples**: ``vfio-pci``, ``mlx5_core``
> +   ``os_driver``          | *string* – this port's device driver when **not** using with DPDK
> +                          | When tearing down the tests on the SUT, DTS will bind the network device
> +                          | *back* to this driver. This driver is meant to be the one that the SUT would
> +                          | normally use for this device, or whichever driver it is preferred to leave the
> +                          | device bound to after testing.
> +

Of note here is that some traffic generators (to which the port config
also applies) won't be using os_driver_for_dpdk (such as Scapy), but
rather os_driver, so the use is broader.

> +                          **Examples**: ``i40e``, ``mlx5_core``
> +   ``peer_node``          *string* – the name of the peer node connected to this port.
> +   ``peer_pci``           *string* – the PCI address of the peer node port. **Example**: ``000a:01:00.1``
> +   ====================== =================================================================================
<snip>
> +Example
> +~~~~~~~
> +
> +The following example (which can be found in ``dts/conf.yaml``) sets up two nodes:
> +
> +* ``SUT1`` which is already setup with the DPDK build requirements and any other
> +  required for execution;
> +* ``TG1`` which already has Scapy installed in the system.
> +
> +And they both have two network ports which are physically connected to each other.
> +
> +.. note::
> +   This example assumes that you have setup SSH keys in both the system under test
> +   and traffic generator nodes.
> +
> +.. literalinclude:: ../../../dts/conf.yaml
> +   :language: yaml
> +   :start-at: executions:
> \ No newline at end of file

The last newline is missing.

> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 37967daea0..2d6fa38a2c 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -1,65 +1,76 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
>
>  executions:
> +  # define one execution environment
>    - build_targets:
>        - arch: x86_64
>          os: linux
>          cpu: native
> +        # the combination of the following two makes CC="ccache gcc"
>          compiler: gcc
>          compiler_wrapper: ccache
> -    perf: false
> -    func: true
> -    skip_smoke_tests: false # optional flag that allows you to skip smoke tests
> -    test_suites:
> +    perf: false # disable performance testing
> +    func: true # enable functional testing
> +    skip_smoke_tests: false

Let's keep the note that the skip_smoke_tests flag is optional

> +    test_suites: # the following test suites will be run in their entirety
>        - hello_world
>        - os_udp
> +    # The machine running the DPDK test executable
>      system_under_test_node:
>        node_name: "SUT 1"
>        vdevs: # optional; if removed, vdevs won't be used in the execution
>          - "crypto_openssl"
> +    # Traffic generator node to use for this execution environment
>      traffic_generator_node: "TG 1"
>  nodes:
> +  # Define a system under test node, having two network ports physically
> +  # connected to the corresponding ports in TG 1 (the peer node)
>    - name: "SUT 1"
>      hostname: sut1.change.me.localhost
>      user: dtsuser
>      arch: x86_64
>      os: linux
> -    lcores: ""
> -    use_first_core: false
> -    memory_channels: 4
> +    lcores: "" # use all the available logical cores
> +    use_first_core: false # tells DPDK to use any physical core
> +    memory_channels: 4 # tells DPDK to use 4 memory channels
>      hugepages:  # optional; if removed, will use system hugepage configuration
>          amount: 256
>          force_first_numa: false
>      ports:
> +      # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
>        - pci: "0000:00:08.0"
>          os_driver_for_dpdk: vfio-pci # OS driver that DPDK will use
> -        os_driver: i40e
> +        os_driver: i40e              # OS driver to bind when the tests are not running
>          peer_node: "TG 1"
>          peer_pci: "0000:00:08.0"
> +      # sets up the physical link between "SUT 1"@000:00:08.1 and "TG 1"@0000:00:08.1
>        - pci: "0000:00:08.1"
>          os_driver_for_dpdk: vfio-pci
>          os_driver: i40e
>          peer_node: "TG 1"
>          peer_pci: "0000:00:08.1"
> +  # Define a Scapy traffic generator node, having two network ports
> +  # physically connected to the corresponding ports in SUT 1 (the peer node).
>    - name: "TG 1"
>      hostname: tg1.change.me.localhost
>      user: dtsuser
>      arch: x86_64
>      os: linux
> -    lcores: ""

The traffic generator may need this core configuration. However, since
it's not required for all traffic generators (such as Scapy), we could
just move to the traffic_generator section. That would require some
code modifications though, but even the removal of lcores and
use_first_core should be addressed in the configuration classes as
well. Have you tried running DTS with these changes?

>      ports:
> +      # sets up the physical link between "TG 1"@000:00:08.0 and "SUT 1"@0000:00:08.0
>        - pci: "0000:00:08.0"
>          os_driver_for_dpdk: rdma
>          os_driver: rdma
>          peer_node: "SUT 1"
>          peer_pci: "0000:00:08.0"
> +      # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
>        - pci: "0000:00:08.1"
>          os_driver_for_dpdk: rdma
>          os_driver: rdma
>          peer_node: "SUT 1"
>          peer_pci: "0000:00:08.1"
> -    use_first_core: false
>      hugepages:  # optional; if removed, will use system hugepage configuration
>          amount: 256
>          force_first_numa: false
> --
> 2.34.1
>


More information about the dev mailing list