Re: [PATCH v2] dts: Testbed And Node Configuration Split

[Jeremy Spewock <jspewock@iol.unh.edu>]

On Wed, Jul 3, 2024 at 10:58 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> This implementation splits the execution and node configuration
> components of the conf.yaml into two separate config files. A
> new command line argument is added, allowing the user to specify
> both a node configuration file and an execution configuration
> file. Be default, these config files are now named node_conf.yaml and
> execution_conf.yaml, respectively.
>
> To assert these changes, the schema calls for one of these objects,
> nodes or executions, in each file, but neither config file can have
> both.
>
> To avoid excess refactoring, both config files are merged together
> early on in the load_config process.
>
> Bugzilla ID: 1344
>
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/execution_conf.yaml                    |  26 ++
>  dts/framework/config/__init__.py           |  26 +-
>  dts/framework/config/conf_yaml_schema.json | 340 ++++++++++-----------
>  dts/framework/runner.py                    |   4 +-
>  dts/framework/settings.py                  |  36 ++-
>  dts/node_conf.yaml                         |  56 ++++
>  dts/testbed_conf.yaml                      |  26 ++
>  7 files changed, 325 insertions(+), 189 deletions(-)
>  create mode 100644 dts/execution_conf.yaml
>  create mode 100644 dts/node_conf.yaml
>  create mode 100644 dts/testbed_conf.yaml
>
> diff --git a/dts/execution_conf.yaml b/dts/execution_conf.yaml
> new file mode 100644
> index 0000000000..af2180eac2
> --- /dev/null
> +++ b/dts/execution_conf.yaml
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
> +
> +executions:

We probably don't want to rename this back to executions as Juraj's
patch recently got merged into main that calls these test_runs, and I
think people found that more clear. With that being said, we'll
probably have to rename this file too.

> +  # 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 # disable performance testing
> +    func: true # enable functional testing
> +    skip_smoke_tests: false # 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"
> \ No newline at end of file
<snip>
> +++ b/dts/node_conf.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
> +
> +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 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 to bind when the tests are not running

I think the point of this comment having all the whitespace is to line
up the starts of these two comments but it seems a little off in this
series. I might even be more of a fan of removing it and just making
it one space, but if we want to keep it the way it is on main it's
probably better to give this a little more whitespace.

> +        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
> +    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"
> +    hugepages:  # optional; if removed, will use system hugepage configuration
> +        amount: 256
> +        force_first_numa: false
> +    traffic_generator:
> +        type: SCAPY
> diff --git a/dts/testbed_conf.yaml b/dts/testbed_conf.yaml

I'm still not sure what keeping this file is for, it seems like it
isn't used elsewhere in the framework.

> new file mode 100644
> index 0000000000..af2180eac2
> --- /dev/null
> +++ b/dts/testbed_conf.yaml
> @@ -0,0 +1,26 @@
> +# 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 # disable performance testing
> +    func: true # enable functional testing
> +    skip_smoke_tests: false # 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"
> \ No newline at end of file
> --
> 2.44.0
>