From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C097843390; Tue, 21 Nov 2023 16:08:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 63AB942EA6; Tue, 21 Nov 2023 16:08:52 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 3D47F42E9D for ; Tue, 21 Nov 2023 16:08:51 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 42E44FEC; Tue, 21 Nov 2023 07:09:37 -0800 (PST) Received: from [10.1.34.179] (e125442.arm.com [10.1.34.179]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 51FCB3F7A6; Tue, 21 Nov 2023 07:08:49 -0800 (PST) Message-ID: <6f683d61-996c-47de-b7f6-e1213a32c4c9@foss.arm.com> Date: Tue, 21 Nov 2023 15:08:47 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 10/21] dts: config docstring update Content-Language: en-US To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com Cc: dev@dpdk.org References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-11-juraj.linkes@pantheon.tech> From: Yoan Picchi In-Reply-To: <20231115130959.39420-11-juraj.linkes@pantheon.tech> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 11/15/23 13:09, Juraj Linkeš wrote: > Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linkeš > --- > dts/framework/config/__init__.py | 371 ++++++++++++++++++++++++++----- > dts/framework/config/types.py | 132 +++++++++++ > 2 files changed, 446 insertions(+), 57 deletions(-) > create mode 100644 dts/framework/config/types.py > > diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py > index 2044c82611..0aa149a53d 100644 > --- a/dts/framework/config/__init__.py > +++ b/dts/framework/config/__init__.py > @@ -3,8 +3,34 @@ > # Copyright(c) 2022-2023 University of New Hampshire > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > -""" > -Yaml config parsing methods > +"""Testbed configuration and test suite specification. > + > +This package offers classes that hold real-time information about the testbed, hold test run > +configuration describing the tested testbed and a loader function, :func:`load_config`, which loads > +the YAML test run configuration file > +and validates it according to :download:`the schema `. > + > +The YAML test run configuration file is parsed into a dictionary, parts of which are used throughout > +this package. The allowed keys and types inside this dictionary are defined in > +the :doc:`types ` module. > + > +The test run configuration has two main sections: > + > + * The :class:`ExecutionConfiguration` which defines what tests are going to be run > + and how DPDK will be built. It also references the testbed where these tests and DPDK > + are going to be run, > + * The nodes of the testbed are defined in the other section, > + a :class:`list` of :class:`NodeConfiguration` objects. > + > +The real-time information about testbed is supposed to be gathered at runtime. > + > +The classes defined in this package make heavy use of :mod:`dataclasses`. > +All of them use slots and are frozen: > + > + * Slots enables some optimizations, by pre-allocating space for the defined > + attributes in the underlying data structure, > + * Frozen makes the object immutable. This enables further optimizations, > + and makes it thread safe should we every want to move in that direction. every -> ever ? > """ > > import json > @@ -12,11 +38,20 @@ > import pathlib > from dataclasses import dataclass > from enum import auto, unique > -from typing import Any, TypedDict, Union > +from typing import Union > > import warlock # type: ignore[import] > import yaml > > +from framework.config.types import ( > + BuildTargetConfigDict, > + ConfigurationDict, > + ExecutionConfigDict, > + NodeConfigDict, > + PortConfigDict, > + TestSuiteConfigDict, > + TrafficGeneratorConfigDict, > +) > from framework.exception import ConfigurationError > from framework.settings import SETTINGS > from framework.utils import StrEnum > @@ -24,55 +59,97 @@ > > @unique > class Architecture(StrEnum): > + r"""The supported architectures of :class:`~framework.testbed_model.node.Node`\s.""" > + > + #: > i686 = auto() > + #: > x86_64 = auto() > + #: > x86_32 = auto() > + #: > arm64 = auto() > + #: > ppc64le = auto() > > > @unique > class OS(StrEnum): > + r"""The supported operating systems of :class:`~framework.testbed_model.node.Node`\s.""" > + > + #: > linux = auto() > + #: > freebsd = auto() > + #: > windows = auto() > > > @unique > class CPUType(StrEnum): > + r"""The supported CPUs of :class:`~framework.testbed_model.node.Node`\s.""" > + > + #: > native = auto() > + #: > armv8a = auto() > + #: > dpaa2 = auto() > + #: > thunderx = auto() > + #: > xgene1 = auto() > > > @unique > class Compiler(StrEnum): > + r"""The supported compilers of :class:`~framework.testbed_model.node.Node`\s.""" > + > + #: > gcc = auto() > + #: > clang = auto() > + #: > icc = auto() > + #: > msvc = auto() > > > @unique > class TrafficGeneratorType(StrEnum): > + """The supported traffic generators.""" > + > + #: > SCAPY = auto() > > > -# Slots enables some optimizations, by pre-allocating space for the defined > -# attributes in the underlying data structure. > -# > -# Frozen makes the object immutable. This enables further optimizations, > -# and makes it thread safe should we every want to move in that direction. > @dataclass(slots=True, frozen=True) > class HugepageConfiguration: > + r"""The hugepage configuration of :class:`~framework.testbed_model.node.Node`\s. > + > + Attributes: > + amount: The number of hugepages. > + force_first_numa: If :data:`True`, the hugepages will be configured on the first NUMA node. > + """ > + > amount: int > force_first_numa: bool > > > @dataclass(slots=True, frozen=True) > class PortConfig: > + r"""The port configuration of :class:`~framework.testbed_model.node.Node`\s. > + > + Attributes: > + node: The :class:`~framework.testbed_model.node.Node` where this port exists. > + pci: The PCI address of the port. > + os_driver_for_dpdk: The operating system driver name for use with DPDK. > + os_driver: The operating system driver name when the operating system controls the port. > + peer_node: The :class:`~framework.testbed_model.node.Node` of the port > + connected to this port. > + peer_pci: The PCI address of the port connected to this port. > + """ > + > node: str > pci: str > os_driver_for_dpdk: str > @@ -81,18 +158,44 @@ class PortConfig: > peer_pci: str > > @staticmethod > - def from_dict(node: str, d: dict) -> "PortConfig": > + def from_dict(node: str, d: PortConfigDict) -> "PortConfig": > + """A convenience method that creates the object from fewer inputs. > + > + Args: > + node: The node where this port exists. > + d: The configuration dictionary. > + > + Returns: > + The port configuration instance. > + """ > return PortConfig(node=node, **d) > > > @dataclass(slots=True, frozen=True) > class TrafficGeneratorConfig: > + """The configuration of traffic generators. > + > + The class will be expanded when more configuration is needed. > + > + Attributes: > + traffic_generator_type: The type of the traffic generator. > + """ > + > traffic_generator_type: TrafficGeneratorType > > @staticmethod > - def from_dict(d: dict) -> "ScapyTrafficGeneratorConfig": > - # This looks useless now, but is designed to allow expansion to traffic > - # generators that require more configuration later. > + def from_dict(d: TrafficGeneratorConfigDict) -> "ScapyTrafficGeneratorConfig": > + """A convenience method that produces traffic generator config of the proper type. > + > + Args: > + d: The configuration dictionary. > + > + Returns: > + The traffic generator configuration instance. > + > + Raises: > + ConfigurationError: An unknown traffic generator type was encountered. > + """ > match TrafficGeneratorType(d["type"]): > case TrafficGeneratorType.SCAPY: > return ScapyTrafficGeneratorConfig( > @@ -106,11 +209,31 @@ def from_dict(d: dict) -> "ScapyTrafficGeneratorConfig": > > @dataclass(slots=True, frozen=True) > class ScapyTrafficGeneratorConfig(TrafficGeneratorConfig): > + """Scapy traffic generator specific configuration.""" > + > pass > > > @dataclass(slots=True, frozen=True) > class NodeConfiguration: > + r"""The configuration of :class:`~framework.testbed_model.node.Node`\s. > + > + Attributes: > + name: The name of the :class:`~framework.testbed_model.node.Node`. > + hostname: The hostname of the :class:`~framework.testbed_model.node.Node`. > + Can be an IP or a domain name. > + user: The name of the user used to connect to > + the :class:`~framework.testbed_model.node.Node`. > + password: The password of the user. The use of passwords is heavily discouraged. > + Please use keys instead. > + arch: The architecture of the :class:`~framework.testbed_model.node.Node`. > + os: The operating system of the :class:`~framework.testbed_model.node.Node`. > + lcores: A comma delimited list of logical cores to use when running DPDK. > + use_first_core: If :data:`True`, the first logical core won't be used. > + hugepages: An optional hugepage configuration. > + ports: The ports that can be used in testing. > + """ > + > name: str > hostname: str > user: str > @@ -123,57 +246,91 @@ class NodeConfiguration: > ports: list[PortConfig] > > @staticmethod > - def from_dict(d: dict) -> Union["SutNodeConfiguration", "TGNodeConfiguration"]: > - hugepage_config = d.get("hugepages") > - if hugepage_config: > - if "force_first_numa" not in hugepage_config: > - hugepage_config["force_first_numa"] = False > - hugepage_config = HugepageConfiguration(**hugepage_config) > - > - common_config = { > - "name": d["name"], > - "hostname": d["hostname"], > - "user": d["user"], > - "password": d.get("password"), > - "arch": Architecture(d["arch"]), > - "os": OS(d["os"]), > - "lcores": d.get("lcores", "1"), > - "use_first_core": d.get("use_first_core", False), > - "hugepages": hugepage_config, > - "ports": [PortConfig.from_dict(d["name"], port) for port in d["ports"]], > - } > - > + def from_dict( > + d: NodeConfigDict, > + ) -> Union["SutNodeConfiguration", "TGNodeConfiguration"]: > + """A convenience method that processes the inputs before creating a specialized instance. > + > + Args: > + d: The configuration dictionary. > + > + Returns: > + Either an SUT or TG configuration instance. > + """ > + hugepage_config = None > + if "hugepages" in d: > + hugepage_config_dict = d["hugepages"] > + if "force_first_numa" not in hugepage_config_dict: > + hugepage_config_dict["force_first_numa"] = False > + hugepage_config = HugepageConfiguration(**hugepage_config_dict) > + > + # The calls here contain duplicated code which is here because Mypy doesn't > + # properly support dictionary unpacking with TypedDicts > if "traffic_generator" in d: > return TGNodeConfiguration( > + name=d["name"], > + hostname=d["hostname"], > + user=d["user"], > + password=d.get("password"), > + arch=Architecture(d["arch"]), > + os=OS(d["os"]), > + lcores=d.get("lcores", "1"), > + use_first_core=d.get("use_first_core", False), > + hugepages=hugepage_config, > + ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]], > traffic_generator=TrafficGeneratorConfig.from_dict( > d["traffic_generator"] > ), > - **common_config, > ) > else: > return SutNodeConfiguration( > - memory_channels=d.get("memory_channels", 1), **common_config > + name=d["name"], > + hostname=d["hostname"], > + user=d["user"], > + password=d.get("password"), > + arch=Architecture(d["arch"]), > + os=OS(d["os"]), > + lcores=d.get("lcores", "1"), > + use_first_core=d.get("use_first_core", False), > + hugepages=hugepage_config, > + ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]], > + memory_channels=d.get("memory_channels", 1), > ) > > > @dataclass(slots=True, frozen=True) > class SutNodeConfiguration(NodeConfiguration): > + """:class:`~framework.testbed_model.sut_node.SutNode` specific configuration. > + > + Attributes: > + memory_channels: The number of memory channels to use when running DPDK. > + """ > + > memory_channels: int > > > @dataclass(slots=True, frozen=True) > class TGNodeConfiguration(NodeConfiguration): > + """:class:`~framework.testbed_model.tg_node.TGNode` specific configuration. > + > + Attributes: > + traffic_generator: The configuration of the traffic generator present on the TG node. > + """ > + > traffic_generator: ScapyTrafficGeneratorConfig > > > @dataclass(slots=True, frozen=True) > class NodeInfo: > - """Class to hold important versions within the node. > - > - This class, unlike the NodeConfiguration class, cannot be generated at the start. > - This is because we need to initialize a connection with the node before we can > - collect the information needed in this class. Therefore, it cannot be a part of > - the configuration class above. > + """Supplemental node information. > + > + Attributes: > + os_name: The name of the running operating system of > + the :class:`~framework.testbed_model.node.Node`. > + os_version: The version of the running operating system of > + the :class:`~framework.testbed_model.node.Node`. > + kernel_version: The kernel version of the running operating system of > + the :class:`~framework.testbed_model.node.Node`. > """ > > os_name: str > @@ -183,6 +340,20 @@ class NodeInfo: > > @dataclass(slots=True, frozen=True) > class BuildTargetConfiguration: > + """DPDK build configuration. > + > + The configuration used for building DPDK. > + > + Attributes: > + arch: The target architecture to build for. > + os: The target os to build for. > + cpu: The target CPU to build for. > + compiler: The compiler executable to use. > + compiler_wrapper: This string will be put in front of the compiler when > + executing the build. Useful for adding wrapper commands, such as ``ccache``. > + name: The name of the compiler. > + """ > + > arch: Architecture > os: OS > cpu: CPUType > @@ -191,7 +362,18 @@ class BuildTargetConfiguration: > name: str > > @staticmethod > - def from_dict(d: dict) -> "BuildTargetConfiguration": > + def from_dict(d: BuildTargetConfigDict) -> "BuildTargetConfiguration": > + r"""A convenience method that processes the inputs before creating an instance. > + > + `arch`, `os`, `cpu` and `compiler` are converted to :class:`Enum`\s and > + `name` is constructed from `arch`, `os`, `cpu` and `compiler`. > + > + Args: > + d: The configuration dictionary. > + > + Returns: > + The build target configuration instance. > + """ > return BuildTargetConfiguration( > arch=Architecture(d["arch"]), > os=OS(d["os"]), > @@ -204,23 +386,29 @@ def from_dict(d: dict) -> "BuildTargetConfiguration": > > @dataclass(slots=True, frozen=True) > class BuildTargetInfo: > - """Class to hold important versions within the build target. > + """Various versions and other information about a build target. > > - This is very similar to the NodeInfo class, it just instead holds information > - for the build target. > + Attributes: > + dpdk_version: The DPDK version that was built. > + compiler_version: The version of the compiler used to build DPDK. > """ > > dpdk_version: str > compiler_version: str > > > -class TestSuiteConfigDict(TypedDict): > - suite: str > - cases: list[str] > - > - > @dataclass(slots=True, frozen=True) > class TestSuiteConfig: > + """Test suite configuration. > + > + Information about a single test suite to be executed. > + > + Attributes: > + test_suite: The name of the test suite module without the starting ``TestSuite_``. > + test_cases: The names of test cases from this test suite to execute. > + If empty, all test cases will be executed. > + """ > + > test_suite: str > test_cases: list[str] > > @@ -228,6 +416,14 @@ class TestSuiteConfig: > def from_dict( > entry: str | TestSuiteConfigDict, > ) -> "TestSuiteConfig": > + """Create an instance from two different types. > + > + Args: > + entry: Either a suite name or a dictionary containing the config. > + > + Returns: > + The test suite configuration instance. > + """ > if isinstance(entry, str): > return TestSuiteConfig(test_suite=entry, test_cases=[]) > elif isinstance(entry, dict): > @@ -238,19 +434,49 @@ def from_dict( > > @dataclass(slots=True, frozen=True) > class ExecutionConfiguration: > + """The configuration of an execution. > + > + The configuration contains testbed information, what tests to execute > + and with what DPDK build. > + > + Attributes: > + build_targets: A list of DPDK builds to test. > + perf: Whether to run performance tests. > + func: Whether to run functional tests. > + skip_smoke_tests: Whether to skip smoke tests. > + test_suites: The names of test suites and/or test cases to execute. > + system_under_test_node: The SUT node to use in this execution. > + traffic_generator_node: The TG node to use in this execution. > + vdevs: The names of virtual devices to test. > + """ > + > build_targets: list[BuildTargetConfiguration] > perf: bool > func: bool > + skip_smoke_tests: bool > test_suites: list[TestSuiteConfig] > system_under_test_node: SutNodeConfiguration > traffic_generator_node: TGNodeConfiguration > vdevs: list[str] > - skip_smoke_tests: bool > > @staticmethod > def from_dict( > - d: dict, node_map: dict[str, Union[SutNodeConfiguration | TGNodeConfiguration]] > + d: ExecutionConfigDict, > + node_map: dict[str, Union[SutNodeConfiguration | TGNodeConfiguration]], > ) -> "ExecutionConfiguration": > + """A convenience method that processes the inputs before creating an instance. > + > + The build target and the test suite config is transformed into their respective objects. is -> are > + SUT and TG configuration are taken from `node_map`. The other (:class:`bool`) attributes are configuration*s* > + just stored. > + > + Args: > + d: The configuration dictionary. > + node_map: A dictionary mapping node names to their config objects. > + > + Returns: > + The execution configuration instance. > + """ > build_targets: list[BuildTargetConfiguration] = list( > map(BuildTargetConfiguration.from_dict, d["build_targets"]) > ) > @@ -291,10 +517,31 @@ def from_dict( > > @dataclass(slots=True, frozen=True) > class Configuration: > + """DTS testbed and test configuration. > + > + The node configuration is not stored in this object. Rather, all used node configurations > + are stored inside the execution configuration where the nodes are actually used. > + > + Attributes: > + executions: Execution configurations. > + """ > + > executions: list[ExecutionConfiguration] > > @staticmethod > - def from_dict(d: dict) -> "Configuration": > + def from_dict(d: ConfigurationDict) -> "Configuration": > + """A convenience method that processes the inputs before creating an instance. > + > + Build target and test suite config is transformed into their respective objects. is -> are > + SUT and TG configuration are taken from `node_map`. The other (:class:`bool`) attributes are configuration*s* > + just stored. > + > + Args: > + d: The configuration dictionary. > + > + Returns: > + The whole configuration instance. > + """ > nodes: list[Union[SutNodeConfiguration | TGNodeConfiguration]] = list( > map(NodeConfiguration.from_dict, d["nodes"]) > ) > @@ -313,9 +560,17 @@ def from_dict(d: dict) -> "Configuration": > > > def load_config() -> Configuration: > - """ > - Loads the configuration file and the configuration file schema, > - validates the configuration file, and creates a configuration object. > + """Load DTS test run configuration from a file. > + > + Load the YAML test run configuration file > + and :download:`the configuration file schema `, > + validate the test run configuration file, and create a test run configuration object. > + > + The YAML test run configuration file is specified in the :option:`--config-file` command line > + argument or the :envvar:`DTS_CFG_FILE` environment variable. > + > + Returns: > + The parsed test run configuration. > """ > with open(SETTINGS.config_file_path, "r") as f: > config_data = yaml.safe_load(f) > @@ -326,6 +581,8 @@ def load_config() -> Configuration: > > with open(schema_path, "r") as f: > schema = json.load(f) > - config: dict[str, Any] = warlock.model_factory(schema, name="_Config")(config_data) > - config_obj: Configuration = Configuration.from_dict(dict(config)) > + config = warlock.model_factory(schema, name="_Config")(config_data) > + config_obj: Configuration = Configuration.from_dict( > + dict(config) # type: ignore[arg-type] > + ) > return config_obj > diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py > new file mode 100644 > index 0000000000..1927910d88 > --- /dev/null > +++ b/dts/framework/config/types.py > @@ -0,0 +1,132 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2023 PANTHEON.tech s.r.o. > + > +"""Configuration dictionary contents specification. > + > +These type definitions serve as documentation of the configuration dictionary contents. > + > +The definitions use the built-in :class:`~typing.TypedDict` construct. > +""" > + > +from typing import TypedDict > + > + > +class PortConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + pci: str > + #: > + os_driver_for_dpdk: str > + #: > + os_driver: str > + #: > + peer_node: str > + #: > + peer_pci: str > + > + > +class TrafficGeneratorConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + type: str > + > + > +class HugepageConfigurationDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + amount: int > + #: > + force_first_numa: bool > + > + > +class NodeConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + hugepages: HugepageConfigurationDict > + #: > + name: str > + #: > + hostname: str > + #: > + user: str > + #: > + password: str > + #: > + arch: str > + #: > + os: str > + #: > + lcores: str > + #: > + use_first_core: bool > + #: > + ports: list[PortConfigDict] > + #: > + memory_channels: int > + #: > + traffic_generator: TrafficGeneratorConfigDict > + > + > +class BuildTargetConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + arch: str > + #: > + os: str > + #: > + cpu: str > + #: > + compiler: str > + #: > + compiler_wrapper: str > + > + > +class TestSuiteConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + suite: str > + #: > + cases: list[str] > + > + > +class ExecutionSUTConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + node_name: str > + #: > + vdevs: list[str] > + > + > +class ExecutionConfigDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + build_targets: list[BuildTargetConfigDict] > + #: > + perf: bool > + #: > + func: bool > + #: > + skip_smoke_tests: bool > + #: > + test_suites: TestSuiteConfigDict > + #: > + system_under_test_node: ExecutionSUTConfigDict > + #: > + traffic_generator_node: str > + > + > +class ConfigurationDict(TypedDict): > + """Allowed keys and values.""" > + > + #: > + nodes: list[NodeConfigDict] > + #: > + executions: list[ExecutionConfigDict]