DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Luca Vizzarro <Luca.Vizzarro@arm.com>
Cc: dev@dpdk.org, Paul Szczepanek <Paul.Szczepanek@arm.com>,
	 Thomas Monjalon <thomas@monjalon.net>,
	Lijuan Tu <lijuan.tu@intel.com>
Subject: Re: [PATCH] dts: improve documentation
Date: Fri, 12 Jan 2024 14:24:56 +0100	[thread overview]
Message-ID: <CAOb5WZYz_NyNAXEiKoATd5GBCfF+mS_cZAa1QaAAYrWg6PBvHA@mail.gmail.com> (raw)
In-Reply-To: <20240103125438.182098-1-Luca.Vizzarro@arm.com>

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
>

  parent reply	other threads:[~2024-01-12 13:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-03 12:54 Luca Vizzarro
2024-01-04 10:52 ` Thomas Monjalon
2024-01-04 12:34   ` Luca Vizzarro
2024-01-04 13:15     ` Thomas Monjalon
2024-01-12 13:24 ` Juraj Linkeš [this message]
2024-01-12 17:16   ` Luca Vizzarro
2024-01-15  9:36     ` Juraj Linkeš
2024-01-15 11:44       ` Luca Vizzarro
2024-01-16 11:44 ` [PATCH v2 1/2] " Luca Vizzarro
2024-01-16 11:44   ` [PATCH v2 2/2] dts: add configuration schema docs Luca Vizzarro
2024-01-16 16:48     ` Juraj Linkeš
2024-01-16 16:47   ` [PATCH v2 1/2] dts: improve documentation Juraj Linkeš
2024-01-19 17:29     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOb5WZYz_NyNAXEiKoATd5GBCfF+mS_cZAa1QaAAYrWg6PBvHA@mail.gmail.com \
    --to=juraj.linkes@pantheon.tech \
    --cc=Luca.Vizzarro@arm.com \
    --cc=Paul.Szczepanek@arm.com \
    --cc=dev@dpdk.org \
    --cc=lijuan.tu@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).