* [PATCH 1/4] dts: Remove build target config and list of devices
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
@ 2024-06-13 20:18 ` Nicholas Pratte
2024-06-14 18:07 ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 2/4] dts: Use First Core Logic Change Nicholas Pratte
` (10 subsequent siblings)
11 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-06-13 20:18 UTC (permalink / raw)
To: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, jspewock, probb, dmarx,
yoan.picchi
Cc: dev, Nicholas Pratte
Remove the list of devices from the schema, as these are unuesed.
Likewise, removed build-target information since these is not currently
used, and it is unlikely to be used in the future. Adjustments to the
dts.rst are made to reflect these changes.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
doc/guides/tools/dts.rst | 11 ---
dts/conf.yaml | 5 +-
dts/framework/config/__init__.py | 30 +-------
dts/framework/config/conf_yaml_schema.json | 79 ----------------------
dts/framework/config/types.py | 6 --
dts/framework/runner.py | 2 +-
dts/framework/test_result.py | 14 +---
dts/framework/testbed_model/sut_node.py | 8 +--
8 files changed, 5 insertions(+), 150 deletions(-)
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 47b218b2c6..da85295db9 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -425,14 +425,6 @@ _`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``.
@@ -444,9 +436,6 @@ _`Build target`
*mapping* – 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.
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 8068345dd5..672e6f92b6 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -5,11 +5,8 @@
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: gcc
compiler_wrapper: ccache
perf: false # disable performance testing
func: true # enable functional testing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 4cb5c74059..5a127a1207 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -84,22 +84,6 @@ class OS(StrEnum):
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."""
@@ -340,28 +324,20 @@ class BuildTargetConfiguration:
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
compiler: Compiler
compiler_wrapper: str
- name: str
@staticmethod
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`.
+ `compiler` is converted to :class:`Enum`\s
Args:
d: The configuration dictionary.
@@ -370,12 +346,8 @@ def from_dict(d: BuildTargetConfigDict) -> "BuildTargetConfiguration":
The build target configuration instance.
"""
return BuildTargetConfiguration(
- arch=Architecture(d["arch"]),
- os=OS(d["os"]),
- cpu=CPUType(d["cpu"]),
compiler=Compiler(d["compiler"]),
compiler_wrapper=d.get("compiler_wrapper", ""),
- name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 4731f4511d..bf0be28176 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,76 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "NIC": {
- "type": "string",
- "enum": [
- "ALL",
- "ConnectX3_MT4103",
- "ConnectX4_LX_MT4117",
- "ConnectX4_MT4115",
- "ConnectX5_MT4119",
- "ConnectX5_MT4121",
- "I40E_10G-10G_BASE_T_BC",
- "I40E_10G-10G_BASE_T_X722",
- "I40E_10G-SFP_X722",
- "I40E_10G-SFP_XL710",
- "I40E_10G-X722_A0",
- "I40E_1G-1G_BASE_T_X722",
- "I40E_25G-25G_SFP28",
- "I40E_40G-QSFP_A",
- "I40E_40G-QSFP_B",
- "IAVF-ADAPTIVE_VF",
- "IAVF-VF",
- "IAVF_10G-X722_VF",
- "ICE_100G-E810C_QSFP",
- "ICE_25G-E810C_SFP",
- "ICE_25G-E810_XXV_SFP",
- "IGB-I350_VF",
- "IGB_1G-82540EM",
- "IGB_1G-82545EM_COPPER",
- "IGB_1G-82571EB_COPPER",
- "IGB_1G-82574L",
- "IGB_1G-82576",
- "IGB_1G-82576_QUAD_COPPER",
- "IGB_1G-82576_QUAD_COPPER_ET2",
- "IGB_1G-82580_COPPER",
- "IGB_1G-I210_COPPER",
- "IGB_1G-I350_COPPER",
- "IGB_1G-I354_SGMII",
- "IGB_1G-PCH_LPTLP_I218_LM",
- "IGB_1G-PCH_LPTLP_I218_V",
- "IGB_1G-PCH_LPT_I217_LM",
- "IGB_1G-PCH_LPT_I217_V",
- "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
- "IGC-I225_LM",
- "IGC-I226_LM",
- "IXGBE_10G-82599_SFP",
- "IXGBE_10G-82599_SFP_SF_QP",
- "IXGBE_10G-82599_T3_LOM",
- "IXGBE_10G-82599_VF",
- "IXGBE_10G-X540T",
- "IXGBE_10G-X540_VF",
- "IXGBE_10G-X550EM_A_SFP",
- "IXGBE_10G-X550EM_X_10G_T",
- "IXGBE_10G-X550EM_X_SFP",
- "IXGBE_10G-X550EM_X_VF",
- "IXGBE_10G-X550T",
- "IXGBE_10G-X550_VF",
- "brcm_57414",
- "brcm_P2100G",
- "cavium_0011",
- "cavium_a034",
- "cavium_a063",
- "cavium_a064",
- "fastlinq_ql41000",
- "fastlinq_ql41000_vf",
- "fastlinq_ql45000",
- "fastlinq_ql45000_vf",
- "hi1822",
- "virtio"
- ]
- },
-
"ARCH": {
"type": "string",
"enum": [
@@ -124,12 +54,6 @@
"other"
]
},
- "os": {
- "$ref": "#/definitions/OS"
- },
- "cpu": {
- "$ref": "#/definitions/cpu"
- },
"compiler": {
"$ref": "#/definitions/compiler"
},
@@ -140,9 +64,6 @@
},
"additionalProperties": false,
"required": [
- "arch",
- "os",
- "cpu",
"compiler"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index 1927910d88..fccea61608 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -74,12 +74,6 @@ class NodeConfigDict(TypedDict):
class BuildTargetConfigDict(TypedDict):
"""Allowed keys and values."""
- #:
- arch: str
- #:
- os: str
- #:
- cpu: str
#:
compiler: str
#:
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index db8e3ba96b..dde008dff5 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -466,7 +466,7 @@ def _run_build_target(
test_suites_with_cases: The test suites with test cases to run.
"""
self._logger.set_stage(DtsStage.build_target_setup)
- self._logger.info(f"Running build target '{build_target.name}'.")
+ self._logger.info("Running build target.")
try:
sut_node.set_up_build_target(build_target)
diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index 28f84fd793..448db86860 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -31,12 +31,9 @@
from typing import Union
from .config import (
- OS,
- Architecture,
BuildTargetConfiguration,
BuildTargetInfo,
Compiler,
- CPUType,
ExecutionConfiguration,
NodeInfo,
TestSuiteConfig,
@@ -223,7 +220,7 @@ class DTSResult(BaseResult):
"""Stores environment information and test results from a DTS run.
* Execution level information, such as testbed and the test suite list,
- * Build target level information, such as compiler, target OS and cpu,
+ * Build target level compiler information
* Test suite and test case results,
* All errors that are caught and recorded during DTS execution.
@@ -403,17 +400,11 @@ class BuildTargetResult(BaseResult):
The internal list stores the results of all test suites in a given build target.
Attributes:
- arch: The DPDK build target architecture.
- os: The DPDK build target operating system.
- cpu: The DPDK build target CPU.
compiler: The DPDK build target compiler.
compiler_version: The DPDK build target compiler version.
dpdk_version: The built DPDK version.
"""
- arch: Architecture
- os: OS
- cpu: CPUType
compiler: Compiler
compiler_version: str | None
dpdk_version: str | None
@@ -431,9 +422,6 @@ def __init__(
build_target: The build target's test run configuration.
"""
super(BuildTargetResult, self).__init__()
- self.arch = build_target.arch
- self.os = build_target.os
- self.cpu = build_target.cpu
self.compiler = build_target.compiler
self.compiler_version = None
self.dpdk_version = None
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 97aa26d419..34213f6884 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -170,12 +170,7 @@ def remote_dpdk_build_dir(self) -> PurePath:
This is the directory where DPDK was built.
We assume it was built in a subdirectory of the extracted tarball.
"""
- if self._build_target_config:
- return self.main_session.join_remote_path(
- self._remote_dpdk_dir, self._build_target_config.name
- )
- else:
- return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
+ return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
@property
def dpdk_version(self) -> str:
@@ -253,7 +248,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
"""Populate common environment variables and set build target config."""
self._env_vars = {}
self._build_target_config = build_target_config
- self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
self._env_vars["CC"] = build_target_config.compiler.name
if build_target_config.compiler_wrapper:
self._env_vars["CC"] = (
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 1/4] dts: Remove build target config and list of devices
2024-06-13 20:18 ` [PATCH 1/4] dts: Remove build target config and list of devices Nicholas Pratte
@ 2024-06-14 18:07 ` Jeremy Spewock
0 siblings, 0 replies; 46+ messages in thread
From: Jeremy Spewock @ 2024-06-14 18:07 UTC (permalink / raw)
To: Nicholas Pratte
Cc: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, probb, dmarx, yoan.picchi, dev
On Thu, Jun 13, 2024 at 4:21 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Remove the list of devices from the schema, as these are unuesed.
> Likewise, removed build-target information since these is not currently
> used, and it is unlikely to be used in the future. Adjustments to the
> dts.rst are made to reflect these changes.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
> doc/guides/tools/dts.rst | 11 ---
> dts/conf.yaml | 5 +-
> dts/framework/config/__init__.py | 30 +-------
> dts/framework/config/conf_yaml_schema.json | 79 ----------------------
> dts/framework/config/types.py | 6 --
> dts/framework/runner.py | 2 +-
> dts/framework/test_result.py | 14 +---
> dts/framework/testbed_model/sut_node.py | 8 +--
> 8 files changed, 5 insertions(+), 150 deletions(-)
>
<snip>
> @unique
> class Compiler(StrEnum):
> r"""The supported compilers of :class:`~framework.testbed_model.node.Node`\s."""
> @@ -340,28 +324,20 @@ class BuildTargetConfiguration:
> 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.
This attribute got removed from the class, we should take it out of
the docstring too.
> """
>
> - arch: Architecture
> - os: OS
> - cpu: CPUType
> compiler: Compiler
> compiler_wrapper: str
> - name: str
>
> @staticmethod
> def from_dict(d: BuildTargetConfigDict) -> "BuildTargetConfiguration":
<snip>
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index 97aa26d419..34213f6884 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
<snip>
> @@ -253,7 +248,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
> """Populate common environment variables and set build target config."""
> self._env_vars = {}
> self._build_target_config = build_target_config
> - self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
I'm not sure what the implications of removing this method call are,
it seems like it adds some value to the DPDK build process by adding a
CFLAG and a path to the pkgconfig directory. Maybe we don't need this,
but there might be a reason it was there originally as well.
> self._env_vars["CC"] = build_target_config.compiler.name
> if build_target_config.compiler_wrapper:
> self._env_vars["CC"] = (
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 2/4] dts: Use First Core Logic Change
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-06-13 20:18 ` [PATCH 1/4] dts: Remove build target config and list of devices Nicholas Pratte
@ 2024-06-13 20:18 ` Nicholas Pratte
2024-06-14 18:09 ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 3/4] dts: Self-Discovering Architecture Change Nicholas Pratte
` (9 subsequent siblings)
11 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-06-13 20:18 UTC (permalink / raw)
To: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, jspewock, probb, dmarx,
yoan.picchi
Cc: dev, Nicholas Pratte
Removed use_first_core from the conf.yaml in favor of determining this
within the framework. use_first_core continue to serve a purpose in that
it is only enabled when core 0 is explicitly provided in the
configuration. Any other configuration, including "" or "any," will
omit core 0.
Documentation reworks are included to reflect the changes made.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
doc/guides/tools/dts.rst | 9 +++------
dts/conf.yaml | 3 +--
dts/framework/config/__init__.py | 11 +++++++----
dts/framework/config/conf_yaml_schema.json | 6 +-----
dts/framework/testbed_model/node.py | 9 +++++++++
5 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index da85295db9..fbb5c6f17b 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -546,15 +546,12 @@ involved in the testing. These can be defined with the following mappings:
+-----------------------+---------------------------------------------------------------------------------------+
| ``os`` | The operating system of this node. See `OS`_ for supported values. |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``lcores`` | | (*optional*, defaults to 1) *string* – Comma-separated list of logical |
- | | | cores to use. An empty string means use all lcores. |
+ | ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
+ | | | cores to use. An empty string means use all lcores except core 0. core 0 is used |
+ | | | only when explicitly specified |
| | |
| | **Example**: ``1,2,3,4,5,18-22`` |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``use_first_core`` | (*optional*, defaults to ``false``) *boolean* |
- | | |
- | | Indicates whether DPDK should use only the first physical core or not. |
- +-----------------------+---------------------------------------------------------------------------------------+
| ``memory_channels`` | (*optional*, defaults to 1) *integer* |
| | |
| | The number of the memory channels to use. |
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 672e6f92b6..c20afb9621 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -29,8 +29,7 @@ nodes:
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
+ lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
hugepages: # optional; if removed, will use system hugepage configuration
amount: 256
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 5a127a1207..6bc290a56a 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -245,6 +245,9 @@ def from_dict(
hugepage_config_dict["force_first_numa"] = False
hugepage_config = HugepageConfiguration(**hugepage_config_dict)
+ lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
+ use_first_core = "0" in lcores
+
# The calls here contain duplicated code which is here because Mypy doesn't
# properly support dictionary unpacking with TypedDicts
if "traffic_generator" in d:
@@ -255,8 +258,8 @@ def from_dict(
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),
+ lcores=lcores,
+ use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
@@ -269,8 +272,8 @@ def from_dict(
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),
+ lcores=lcores,
+ use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
memory_channels=d.get("memory_channels", 1),
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index bf0be28176..7c8429abbc 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -163,13 +163,9 @@
},
"lcores": {
"type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
"description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
},
- "use_first_core": {
- "type": "boolean",
- "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
- },
"memory_channels": {
"type": "integer",
"description": "How many memory channels to use. Optional, defaults to 1."
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 74061f6262..470cd18e30 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -93,6 +93,15 @@ def __init__(self, node_config: NodeConfiguration):
self.lcores, LogicalCoreList(self.config.lcores)
).filter()
+ if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
+ self._logger.info(
+ """
+ WARNING: First core being used;
+ using the first core is considered risky and should only
+ be done by advanced users.
+ """
+ )
+
self._other_sessions = []
self.virtual_devices = []
self._init_ports()
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 2/4] dts: Use First Core Logic Change
2024-06-13 20:18 ` [PATCH 2/4] dts: Use First Core Logic Change Nicholas Pratte
@ 2024-06-14 18:09 ` Jeremy Spewock
2024-06-20 13:41 ` Nicholas Pratte
0 siblings, 1 reply; 46+ messages in thread
From: Jeremy Spewock @ 2024-06-14 18:09 UTC (permalink / raw)
To: Nicholas Pratte
Cc: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, probb, dmarx, yoan.picchi, dev
On Thu, Jun 13, 2024 at 4:21 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Removed use_first_core from the conf.yaml in favor of determining this
> within the framework. use_first_core continue to serve a purpose in that
> it is only enabled when core 0 is explicitly provided in the
> configuration. Any other configuration, including "" or "any," will
> omit core 0.
>
> Documentation reworks are included to reflect the changes made.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
> doc/guides/tools/dts.rst | 9 +++------
> dts/conf.yaml | 3 +--
> dts/framework/config/__init__.py | 11 +++++++----
> dts/framework/config/conf_yaml_schema.json | 6 +-----
> dts/framework/testbed_model/node.py | 9 +++++++++
> 5 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> index da85295db9..fbb5c6f17b 100644
> --- a/doc/guides/tools/dts.rst
> +++ b/doc/guides/tools/dts.rst
> @@ -546,15 +546,12 @@ involved in the testing. These can be defined with the following mappings:
> +-----------------------+---------------------------------------------------------------------------------------+
> | ``os`` | The operating system of this node. See `OS`_ for supported values. |
> +-----------------------+---------------------------------------------------------------------------------------+
> - | ``lcores`` | | (*optional*, defaults to 1) *string* – Comma-separated list of logical |
> - | | | cores to use. An empty string means use all lcores. |
> + | ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
I think just leaving this as "defaults to 1" is fine here. It's more
explicit to say "if it isn't used", but just saying it defaults I
think implies that enough.
> + | | | cores to use. An empty string means use all lcores except core 0. core 0 is used |
> + | | | only when explicitly specified |
> | | |
> | | **Example**: ``1,2,3,4,5,18-22`` |
> +-----------------------+---------------------------------------------------------------------------------------+
> - | ``use_first_core`` | (*optional*, defaults to ``false``) *boolean* |
> - | | |
> - | | Indicates whether DPDK should use only the first physical core or not. |
> - +-----------------------+---------------------------------------------------------------------------------------+
> | ``memory_channels`` | (*optional*, defaults to 1) *integer* |
> | | |
> | | The number of the memory channels to use. |
<snip>
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index 5a127a1207..6bc290a56a 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -245,6 +245,9 @@ def from_dict(
> hugepage_config_dict["force_first_numa"] = False
> hugepage_config = HugepageConfiguration(**hugepage_config_dict)
>
> + lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
> + use_first_core = "0" in lcores
> +
> # The calls here contain duplicated code which is here because Mypy doesn't
> # properly support dictionary unpacking with TypedDicts
> if "traffic_generator" in d:
> @@ -255,8 +258,8 @@ def from_dict(
> 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),
> + lcores=lcores,
> + use_first_core=use_first_core,
I wonder if we could completely remove the "use_first_core" attribute
from the config classes. It seems like it's only used when you're
getting remote CPUs to skip core 0 based on the condition. With these
new changes it seems like we just assume that if 0 is in the list, we
will use it, otherwise we simply won't, so I don't think we need this
condition to detect whether we should skip or not anymore, do we?
> hugepages=hugepage_config,
> ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
> traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
> @@ -269,8 +272,8 @@ def from_dict(
> 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),
> + lcores=lcores,
> + use_first_core=use_first_core,
> hugepages=hugepage_config,
> ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
> memory_channels=d.get("memory_channels", 1),
<snip>
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> index 74061f6262..470cd18e30 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -93,6 +93,15 @@ def __init__(self, node_config: NodeConfiguration):
> self.lcores, LogicalCoreList(self.config.lcores)
> ).filter()
>
> + if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
This condition fits if you completely remove the `use_first_core`
boolean from the NodeConfiguration, but if we decide not to I think it
could be shortened to just:
if config.use_first_core:
> + self._logger.info(
> + """
> + WARNING: First core being used;
> + using the first core is considered risky and should only
> + be done by advanced users.
> + """
> + )
> +
> self._other_sessions = []
> self.virtual_devices = []
> self._init_ports()
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 2/4] dts: Use First Core Logic Change
2024-06-14 18:09 ` Jeremy Spewock
@ 2024-06-20 13:41 ` Nicholas Pratte
0 siblings, 0 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-06-20 13:41 UTC (permalink / raw)
To: Jeremy Spewock
Cc: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, probb, dmarx, yoan.picchi, dev
On Fri, Jun 14, 2024 at 2:09 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> On Thu, Jun 13, 2024 at 4:21 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> >
> > Removed use_first_core from the conf.yaml in favor of determining this
> > within the framework. use_first_core continue to serve a purpose in that
> > it is only enabled when core 0 is explicitly provided in the
> > configuration. Any other configuration, including "" or "any," will
> > omit core 0.
> >
> > Documentation reworks are included to reflect the changes made.
> >
> > Bugzilla ID: 1360
> > Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> >
> > ---
> > doc/guides/tools/dts.rst | 9 +++------
> > dts/conf.yaml | 3 +--
> > dts/framework/config/__init__.py | 11 +++++++----
> > dts/framework/config/conf_yaml_schema.json | 6 +-----
> > dts/framework/testbed_model/node.py | 9 +++++++++
> > 5 files changed, 21 insertions(+), 17 deletions(-)
> >
> > diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> > index da85295db9..fbb5c6f17b 100644
> > --- a/doc/guides/tools/dts.rst
> > +++ b/doc/guides/tools/dts.rst
> > @@ -546,15 +546,12 @@ involved in the testing. These can be defined with the following mappings:
> > +-----------------------+---------------------------------------------------------------------------------------+
> > | ``os`` | The operating system of this node. See `OS`_ for supported values. |
> > +-----------------------+---------------------------------------------------------------------------------------+
> > - | ``lcores`` | | (*optional*, defaults to 1) *string* – Comma-separated list of logical |
> > - | | | cores to use. An empty string means use all lcores. |
> > + | ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
>
> I think just leaving this as "defaults to 1" is fine here. It's more
> explicit to say "if it isn't used", but just saying it defaults I
> think implies that enough.
Good point, and you're right. '*optional* and 'if not used' are redundant.
>
> > + | | | cores to use. An empty string means use all lcores except core 0. core 0 is used |
> > + | | | only when explicitly specified |
> > | | |
> > | | **Example**: ``1,2,3,4,5,18-22`` |
> > +-----------------------+---------------------------------------------------------------------------------------+
> > - | ``use_first_core`` | (*optional*, defaults to ``false``) *boolean* |
> > - | | |
> > - | | Indicates whether DPDK should use only the first physical core or not. |
> > - +-----------------------+---------------------------------------------------------------------------------------+
> > | ``memory_channels`` | (*optional*, defaults to 1) *integer* |
> > | | |
> > | | The number of the memory channels to use. |
> <snip>
> > diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> > index 5a127a1207..6bc290a56a 100644
> > --- a/dts/framework/config/__init__.py
> > +++ b/dts/framework/config/__init__.py
> > @@ -245,6 +245,9 @@ def from_dict(
> > hugepage_config_dict["force_first_numa"] = False
> > hugepage_config = HugepageConfiguration(**hugepage_config_dict)
> >
> > + lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
> > + use_first_core = "0" in lcores
> > +
> > # The calls here contain duplicated code which is here because Mypy doesn't
> > # properly support dictionary unpacking with TypedDicts
> > if "traffic_generator" in d:
> > @@ -255,8 +258,8 @@ def from_dict(
> > 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),
> > + lcores=lcores,
> > + use_first_core=use_first_core,
>
> I wonder if we could completely remove the "use_first_core" attribute
> from the config classes. It seems like it's only used when you're
> getting remote CPUs to skip core 0 based on the condition. With these
> new changes it seems like we just assume that if 0 is in the list, we
> will use it, otherwise we simply won't, so I don't think we need this
> condition to detect whether we should skip or not anymore, do we?
It's an interesting point, and it's one that I honestly thought about
right after I completed the last patch in this series (DPDK_config
config attribute). It wasn't until I tried implementing the
'dpdk_config' attribute that I realized the broader scope of lcore
filtering in the framework. The framework is currently filtering a
list of lcores in two spots of the framework. The last patch, as you
saw, addresses the aforementioned problem by removing 'use_first_core'
from the framework completely.
In retrospect, I could have dropped this commit and just included the
'dpdk_config' patch with the 'use_first_core' changes included, but I
suppose it's not bad for others to see both implementations and
discuss what the better option is.
>
> > hugepages=hugepage_config,
> > ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
> > traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
> > @@ -269,8 +272,8 @@ def from_dict(
> > 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),
> > + lcores=lcores,
> > + use_first_core=use_first_core,
> > hugepages=hugepage_config,
> > ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
> > memory_channels=d.get("memory_channels", 1),
> <snip>
> > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> > index 74061f6262..470cd18e30 100644
> > --- a/dts/framework/testbed_model/node.py
> > +++ b/dts/framework/testbed_model/node.py
> > @@ -93,6 +93,15 @@ def __init__(self, node_config: NodeConfiguration):
> > self.lcores, LogicalCoreList(self.config.lcores)
> > ).filter()
> >
> > + if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
>
> This condition fits if you completely remove the `use_first_core`
> boolean from the NodeConfiguration, but if we decide not to I think it
> could be shortened to just:
>
> if config.use_first_core:
Noted, and good catch! I'll keep this in mind depending on which way
the wind blows.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 3/4] dts: Self-Discovering Architecture Change
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-06-13 20:18 ` [PATCH 1/4] dts: Remove build target config and list of devices Nicholas Pratte
2024-06-13 20:18 ` [PATCH 2/4] dts: Use First Core Logic Change Nicholas Pratte
@ 2024-06-13 20:18 ` Nicholas Pratte
2024-06-14 18:09 ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
` (8 subsequent siblings)
11 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-06-13 20:18 UTC (permalink / raw)
To: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, jspewock, probb, dmarx,
yoan.picchi
Cc: dev, Nicholas Pratte
The 'arch' attribute in the conf.yaml is unnecessary, as this can be
readily discovered within the constructor of any given node. Since OS is
determined within user configuration, finding system arch can be done
both reliably and easily within the framework.
For Linux/Posix systems, the 'uname' command is used to determine system
architecture. I believe that this is posix-standard and utilizes a
standardized output.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
doc/guides/tools/dts.rst | 2 --
dts/conf.yaml | 2 --
dts/framework/config/__init__.py | 4 ----
dts/framework/config/conf_yaml_schema.json | 12 ------------
dts/framework/config/types.py | 2 --
dts/framework/testbed_model/node.py | 3 +++
dts/framework/testbed_model/os_session.py | 8 ++++++++
dts/framework/testbed_model/posix_session.py | 6 ++++++
8 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index fbb5c6f17b..0453a15a73 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -542,8 +542,6 @@ involved in the testing. These can be defined with the following mappings:
| | |
| | **NB**: Use only as last resort. SSH keys are **strongly** preferred. |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``arch`` | The architecture of this node. See `ARCH`_ for supported values. |
- +-----------------------+---------------------------------------------------------------------------------------+
| ``os`` | The operating system of this node. See `OS`_ for supported values. |
+-----------------------+---------------------------------------------------------------------------------------+
| ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
diff --git a/dts/conf.yaml b/dts/conf.yaml
index c20afb9621..b9f5704ca5 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -27,7 +27,6 @@ nodes:
- name: "SUT 1"
hostname: sut1.change.me.localhost
user: dtsuser
- arch: x86_64
os: linux
lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
@@ -52,7 +51,6 @@ nodes:
- 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
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 6bc290a56a..07b85a6afb 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -207,7 +207,6 @@ class NodeConfiguration:
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.
@@ -219,7 +218,6 @@ class NodeConfiguration:
hostname: str
user: str
password: str | None
- arch: Architecture
os: OS
lcores: str
use_first_core: bool
@@ -256,7 +254,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
@@ -270,7 +267,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 7c8429abbc..49db384967 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,14 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "ARCH": {
- "type": "string",
- "enum": [
- "x86_64",
- "arm64",
- "ppc64le"
- ]
- },
"OS": {
"type": "string",
"enum": [
@@ -155,9 +147,6 @@
"type": "string",
"description": "The password to use on this node. Use only as a last resort. SSH keys are STRONGLY preferred."
},
- "arch": {
- "$ref": "#/definitions/ARCH"
- },
"os": {
"$ref": "#/definitions/OS"
},
@@ -233,7 +222,6 @@
"name",
"hostname",
"user",
- "arch",
"os"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index fccea61608..c841ab2d7c 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -56,8 +56,6 @@ class NodeConfigDict(TypedDict):
#:
password: str
#:
- arch: str
- #:
os: str
#:
lcores: str
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 470cd18e30..ee4577cf35 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -18,6 +18,7 @@
from framework.config import (
OS,
+ Architecture,
BuildTargetConfiguration,
ExecutionConfiguration,
NodeConfiguration,
@@ -61,6 +62,7 @@ class Node(ABC):
main_session: OSSession
config: NodeConfiguration
name: str
+ arch: Architecture
lcores: list[LogicalCore]
ports: list[Port]
_logger: DTSLogger
@@ -84,6 +86,7 @@ def __init__(self, node_config: NodeConfiguration):
self.name = node_config.name
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
+ self.arch = Architecture(self.main_session.get_arch_info())
self._logger.info(f"Connected to node: {self.name}")
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index d5bf7e0401..e082102b00 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -375,6 +375,14 @@ def get_node_info(self) -> NodeInfo:
Node information.
"""
+ @abstractmethod
+ def get_arch_info(self) -> str:
+ """Discover CPU architecture of the remote host.
+
+ Returns:
+ Remote host CPU architecture.
+ """
+
@abstractmethod
def update_ports(self, ports: list[Port]) -> None:
"""Get additional information about ports from the operating system and update them.
diff --git a/dts/framework/testbed_model/posix_session.py b/dts/framework/testbed_model/posix_session.py
index d279bb8b53..91afca61ea 100644
--- a/dts/framework/testbed_model/posix_session.py
+++ b/dts/framework/testbed_model/posix_session.py
@@ -295,3 +295,9 @@ def get_node_info(self) -> NodeInfo:
).stdout.split("\n")
kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
+
+ def get_arch_info(self) -> str:
+ """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
+ # return str(self.send_command('arch')).stdout
+
+ return str(self.send_command("uname -m").stdout.removesuffix("\n"))
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 3/4] dts: Self-Discovering Architecture Change
2024-06-13 20:18 ` [PATCH 3/4] dts: Self-Discovering Architecture Change Nicholas Pratte
@ 2024-06-14 18:09 ` Jeremy Spewock
0 siblings, 0 replies; 46+ messages in thread
From: Jeremy Spewock @ 2024-06-14 18:09 UTC (permalink / raw)
To: Nicholas Pratte
Cc: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, probb, dmarx, yoan.picchi, dev
Looks good to me, just looks like a testing command got left behind.
Otherwise though:
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
On Thu, Jun 13, 2024 at 4:22 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The 'arch' attribute in the conf.yaml is unnecessary, as this can be
> readily discovered within the constructor of any given node. Since OS is
> determined within user configuration, finding system arch can be done
> both reliably and easily within the framework.
>
> For Linux/Posix systems, the 'uname' command is used to determine system
> architecture. I believe that this is posix-standard and utilizes a
> standardized output.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
<snip>
> diff --git a/dts/framework/testbed_model/posix_session.py b/dts/framework/testbed_model/posix_session.py
> index d279bb8b53..91afca61ea 100644
> --- a/dts/framework/testbed_model/posix_session.py
> +++ b/dts/framework/testbed_model/posix_session.py
> @@ -295,3 +295,9 @@ def get_node_info(self) -> NodeInfo:
> ).stdout.split("\n")
> kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
> return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
> +
> + def get_arch_info(self) -> str:
> + """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
> + # return str(self.send_command('arch')).stdout
Right here is the testing I was referencing.
> +
> + return str(self.send_command("uname -m").stdout.removesuffix("\n"))
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (2 preceding siblings ...)
2024-06-13 20:18 ` [PATCH 3/4] dts: Self-Discovering Architecture Change Nicholas Pratte
@ 2024-06-13 20:18 ` Nicholas Pratte
2024-06-14 18:11 ` Jeremy Spewock
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (7 subsequent siblings)
11 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-06-13 20:18 UTC (permalink / raw)
To: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, jspewock, probb, dmarx,
yoan.picchi
Cc: dev, Nicholas Pratte
Rework 'lcores' and 'memory_channels' into a new 'dpdk_config'
subsection in an effort to make these attributes SUT specific; the
traffic generator, more often than not, does not need this information.
Ideally, if such information is needed, then it will be listed in the
'traffic_generator' component in TG Node configuration. Such logic is
not introduced in this patch, but the framework can be rewritten to do
so without any implications of extreme effort.
To make this work, use_first_core has been removed from the framework
entirely in favor of doing this within the LogicalCoreListFilter object.
Since use_first_core was only ever activated when logical core 0 was
explicitly defined, core 0 can be removed from the list of total logical
cores assuming that it was not listed within filter_specifier.
This patch also removes 'vdevs' from 'system_under_test_node' and moves
it into 'executions.'
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
doc/guides/tools/dts.rst | 2 +
dts/conf.yaml | 18 +++----
dts/framework/config/__init__.py | 45 +++++++++++-------
dts/framework/config/conf_yaml_schema.json | 49 ++++++++++----------
dts/framework/config/types.py | 30 ++++++------
dts/framework/testbed_model/cpu.py | 5 ++
dts/framework/testbed_model/linux_session.py | 5 +-
dts/framework/testbed_model/node.py | 26 +----------
dts/framework/testbed_model/os_session.py | 2 +-
dts/framework/testbed_model/sut_node.py | 17 ++++++-
10 files changed, 100 insertions(+), 99 deletions(-)
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 0453a15a73..9d780d5dcd 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -544,6 +544,8 @@ involved in the testing. These can be defined with the following mappings:
+-----------------------+---------------------------------------------------------------------------------------+
| ``os`` | The operating system of this node. See `OS`_ for supported values. |
+-----------------------+---------------------------------------------------------------------------------------+
+ | ``dpdk_config`` | Configuration relating to DPDK (to be specified on SUT Nodes) |
+ +-----------------------+---------------------------------------------------------------------------------------+
| ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
| | | cores to use. An empty string means use all lcores except core 0. core 0 is used |
| | | only when explicitly specified |
diff --git a/dts/conf.yaml b/dts/conf.yaml
index b9f5704ca5..b7a2ed567d 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -14,11 +14,10 @@ executions:
test_suites: # the following test suites will be run in their entirety
- hello_world
- os_udp
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
# 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"
+ system_under_test_node: "SUT 1"
# Traffic generator node to use for this execution environment
traffic_generator_node: "TG 1"
nodes:
@@ -28,11 +27,6 @@ nodes:
hostname: sut1.change.me.localhost
user: dtsuser
os: linux
- lcores: "" # use all available logical cores (Skips first 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"
@@ -46,6 +40,12 @@ nodes:
os_driver: i40e
peer_node: "TG 1"
peer_pci: "0000:00:08.1"
+ hugepages: # optional; if removed, will use system hugepage configuration
+ amount: 256
+ force_first_numa: false
+ dpdk_config:
+ lcores: "" # use all available logical cores (Skips first core)
+ memory_channels: 4 # tells DPDK to use 4 memory channels
# 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"
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 07b85a6afb..9ca70b3fdd 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -208,8 +208,6 @@ class NodeConfiguration:
password: The password of the user. The use of passwords is heavily discouraged.
Please use keys instead.
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.
"""
@@ -219,8 +217,6 @@ class NodeConfiguration:
user: str
password: str | None
os: OS
- lcores: str
- use_first_core: bool
hugepages: HugepageConfiguration | None
ports: list[PortConfig]
@@ -243,9 +239,6 @@ def from_dict(
hugepage_config_dict["force_first_numa"] = False
hugepage_config = HugepageConfiguration(**hugepage_config_dict)
- lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
- use_first_core = "0" in lcores
-
# The calls here contain duplicated code which is here because Mypy doesn't
# properly support dictionary unpacking with TypedDicts
if "traffic_generator" in d:
@@ -255,36 +248,54 @@ def from_dict(
user=d["user"],
password=d.get("password"),
os=OS(d["os"]),
- lcores=lcores,
- use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
)
else:
+ dpdk_config = d["dpdk_config"]
+ dpdk_config["lcores"] = (
+ "1"
+ if "lcores" not in dpdk_config
+ else dpdk_config["lcores"]
+ if "any" not in dpdk_config["lcores"]
+ else ""
+ )
+ dpdk_config["memory_channels"] = dpdk_config.get("memory_channels", 1)
return SutNodeConfiguration(
name=d["name"],
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
os=OS(d["os"]),
- lcores=lcores,
- use_first_core=use_first_core,
+ dpdk_config=DPDKConfig(**dpdk_config),
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 DPDKConfig:
+ """EAL parameters for executing and running DPDK.
+
+ Attributes:
+ lcores: Logical cores to be used for DPDK execution.
+ memory_channels: Memory channels to be used for DPDK execution.
+ """
+
+ lcores: str
+ memory_channels: int
+
+
@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.
+ dpdk_config: DPDK configuration attributes to be used during execution.
"""
- memory_channels: int
+ dpdk_config: DPDKConfig
@dataclass(slots=True, frozen=True)
@@ -447,7 +458,7 @@ def from_dict(
map(BuildTargetConfiguration.from_dict, d["build_targets"])
)
test_suites: list[TestSuiteConfig] = list(map(TestSuiteConfig.from_dict, d["test_suites"]))
- sut_name = d["system_under_test_node"]["node_name"]
+ sut_name = d["system_under_test_node"]
skip_smoke_tests = d.get("skip_smoke_tests", False)
assert sut_name in node_map, f"Unknown SUT {sut_name} in execution {d}"
system_under_test_node = node_map[sut_name]
@@ -462,9 +473,7 @@ def from_dict(
traffic_generator_node, TGNodeConfiguration
), f"Invalid TG configuration {traffic_generator_node}"
- vdevs = (
- d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
- )
+ vdevs = d["vdevs"] if "vdevs" in d else []
return ExecutionConfiguration(
build_targets=build_targets,
perf=d["perf"],
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 49db384967..58e308a76c 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -150,18 +150,25 @@
"os": {
"$ref": "#/definitions/OS"
},
- "lcores": {
- "type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
- "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
- },
- "memory_channels": {
- "type": "integer",
- "description": "How many memory channels to use. Optional, defaults to 1."
- },
"hugepages": {
"$ref": "#/definitions/hugepages"
},
+ "dpdk_config": {
+ "type": "object",
+ "description": "EAL arguments for DPDK execution",
+ "properties": {
+ "lcores": {
+ "type": "string",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
+ "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
+ },
+ "memory_channels": {
+ "type": "integer",
+ "description": "How many memory channels to use. Optional, defaults to 1."
+ }
+ },
+ "minimum": 1
+ },
"ports": {
"type": "array",
"items": {
@@ -264,23 +271,15 @@
"description": "Optional field that allows you to skip smoke testing",
"type": "boolean"
},
+ "vdevs": {
+ "description": "Optional list of names of vdevs to be used in execution",
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
"system_under_test_node": {
- "type":"object",
- "properties": {
- "node_name": {
- "$ref": "#/definitions/node_name"
- },
- "vdevs": {
- "description": "Optional list of names of vdevs to be used in execution",
- "type": "array",
- "items": {
- "type": "string"
- }
- }
- },
- "required": [
- "node_name"
- ]
+ "$ref": "#/definitions/node_name"
},
"traffic_generator_node": {
"$ref": "#/definitions/node_name"
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index c841ab2d7c..fb760daa9f 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -33,6 +33,15 @@ class TrafficGeneratorConfigDict(TypedDict):
type: str
+class DPDKConfigDict(TypedDict):
+ """Allowed keys and values."""
+
+ #:
+ memory_channels: int
+ #:
+ lcores: str
+
+
class HugepageConfigurationDict(TypedDict):
"""Allowed keys and values."""
@@ -58,15 +67,11 @@ class NodeConfigDict(TypedDict):
#:
os: str
#:
- lcores: str
- #:
- use_first_core: bool
- #:
ports: list[PortConfigDict]
#:
- memory_channels: int
- #:
traffic_generator: TrafficGeneratorConfigDict
+ #:
+ dpdk_config: DPDKConfigDict
class BuildTargetConfigDict(TypedDict):
@@ -87,15 +92,6 @@ class TestSuiteConfigDict(TypedDict):
cases: list[str]
-class ExecutionSUTConfigDict(TypedDict):
- """Allowed keys and values."""
-
- #:
- node_name: str
- #:
- vdevs: list[str]
-
-
class ExecutionConfigDict(TypedDict):
"""Allowed keys and values."""
@@ -110,9 +106,11 @@ class ExecutionConfigDict(TypedDict):
#:
test_suites: TestSuiteConfigDict
#:
- system_under_test_node: ExecutionSUTConfigDict
+ system_under_test_node: str
#:
traffic_generator_node: str
+ #:
+ vdevs: list[str]
class ConfigurationDict(TypedDict):
diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py
index 9e33b2825d..0c315a0da6 100644
--- a/dts/framework/testbed_model/cpu.py
+++ b/dts/framework/testbed_model/cpu.py
@@ -210,6 +210,8 @@ def filter(self) -> list[LogicalCore]:
Returns:
The filtered cores.
"""
+ if 0 in self._lcores_to_filter:
+ self._lcores_to_filter = self._lcores_to_filter[1:]
sockets_to_filter = self._filter_sockets(self._lcores_to_filter)
filtered_lcores = []
for socket_to_filter in sockets_to_filter:
@@ -328,6 +330,9 @@ def filter(self) -> list[LogicalCore]:
Return:
The filtered logical CPU cores.
"""
+ if 0 not in self._filter_specifier.lcore_list:
+ self._lcores_to_filter = self._lcores_to_filter[1:]
+
if not len(self._filter_specifier.lcore_list):
return self._lcores_to_filter
diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 5d24030c3d..9d887ef6db 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -68,15 +68,12 @@ class LinuxSession(PosixSession):
def _get_privileged_command(command: str) -> str:
return f"sudo -- sh -c '{command}'"
- def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
+ def get_remote_cpus(self) -> list[LogicalCore]:
"""Overrides :meth:`~.os_session.OSSession.get_remote_cpus`."""
cpu_info = self.send_command("lscpu -p=CPU,CORE,SOCKET,NODE|grep -v \\#").stdout
lcores = []
for cpu_line in cpu_info.splitlines():
lcore, core, socket, node = map(int, cpu_line.split(","))
- if core == 0 and socket == 0 and not use_first_core:
- self._logger.info("Not using the first physical core.")
- continue
lcores.append(LogicalCore(lcore, core, socket, node))
return lcores
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index ee4577cf35..19be6c8c4f 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -27,13 +27,7 @@
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
-from .cpu import (
- LogicalCore,
- LogicalCoreCount,
- LogicalCoreList,
- LogicalCoreListFilter,
- lcore_filter,
-)
+from .cpu import LogicalCore, LogicalCoreCount, LogicalCoreList, lcore_filter
from .linux_session import LinuxSession
from .os_session import InteractiveShellType, OSSession
from .port import Port
@@ -87,24 +81,8 @@ def __init__(self, node_config: NodeConfiguration):
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
self.arch = Architecture(self.main_session.get_arch_info())
-
self._logger.info(f"Connected to node: {self.name}")
-
self._get_remote_cpus()
- # filter the node lcores according to the test run configuration
- self.lcores = LogicalCoreListFilter(
- self.lcores, LogicalCoreList(self.config.lcores)
- ).filter()
-
- if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
- self._logger.info(
- """
- WARNING: First core being used;
- using the first core is considered risky and should only
- be done by advanced users.
- """
- )
-
self._other_sessions = []
self.virtual_devices = []
self._init_ports()
@@ -269,7 +247,7 @@ def filter_lcores(
def _get_remote_cpus(self) -> None:
"""Scan CPUs in the remote OS and store a list of LogicalCores."""
self._logger.info("Getting CPU information.")
- self.lcores = self.main_session.get_remote_cpus(self.config.use_first_core)
+ self.lcores = self.main_session.get_remote_cpus()
def _setup_hugepages(self) -> None:
"""Setup hugepages on the node.
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index e082102b00..07b0189368 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -314,7 +314,7 @@ def get_dpdk_version(self, version_path: str | PurePath) -> str:
"""
@abstractmethod
- def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
+ def get_remote_cpus(self) -> list[LogicalCore]:
r"""Get the list of :class:`~.cpu.LogicalCore`\s on the remote node.
Args:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 34213f6884..741d8f3cea 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -27,7 +27,7 @@
from framework.settings import SETTINGS
from framework.utils import MesonArgs
-from .cpu import LogicalCoreCount, LogicalCoreList
+from .cpu import LogicalCore, LogicalCoreCount, LogicalCoreList, LogicalCoreListFilter
from .node import Node
from .os_session import InteractiveShellType, OSSession
from .port import Port
@@ -131,6 +131,19 @@ def __init__(self, node_config: SutNodeConfiguration):
node_config: The SUT node's test run configuration.
"""
super(SutNode, self).__init__(node_config)
+ self.lcores = LogicalCoreListFilter(
+ self.lcores, LogicalCoreList(self.config.dpdk_config.lcores)
+ ).filter()
+ if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
+ self._logger.info(
+ """
+ WARNING: First core being used;
+ using the first core is considered risky and should only
+ be done by advanced users.
+ """
+ )
+ else:
+ self._logger.info("Not using first core")
self._dpdk_prefix_list = []
self._build_target_config = None
self._env_vars = {}
@@ -395,7 +408,7 @@ def create_eal_parameters(
return EalParameters(
lcore_list=lcore_list,
- memory_channels=self.config.memory_channels,
+ memory_channels=self.config.dpdk_config.memory_channels,
prefix=prefix,
no_pci=no_pci,
vdevs=vdevs,
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config
2024-06-13 20:18 ` [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
@ 2024-06-14 18:11 ` Jeremy Spewock
0 siblings, 0 replies; 46+ messages in thread
From: Jeremy Spewock @ 2024-06-14 18:11 UTC (permalink / raw)
To: Nicholas Pratte
Cc: Honnappa.Nagarahalli, paul.szczepanek, luca.vizzarro,
juraj.linkes, bruce.richardson, probb, dmarx, yoan.picchi, dev
Funny, this commit addresses a comment I had on the previous. I think
it makes a lot of sense to split the EAL parameter information into a
DPDK specific config that the TG doesn't have since it likely won't
need it.
On Thu, Jun 13, 2024 at 4:22 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Rework 'lcores' and 'memory_channels' into a new 'dpdk_config'
> subsection in an effort to make these attributes SUT specific; the
> traffic generator, more often than not, does not need this information.
> Ideally, if such information is needed, then it will be listed in the
> 'traffic_generator' component in TG Node configuration. Such logic is
> not introduced in this patch, but the framework can be rewritten to do
> so without any implications of extreme effort.
>
> To make this work, use_first_core has been removed from the framework
I think it makes more sense to do this in the commit where you removed
it from the config as well and just completely take it out. There
isn't really a need to keep it in the framework in that commit so I'd
be more in favor of removing it from there entirely and then this
commit won't need to since it's less relevant here.
> entirely in favor of doing this within the LogicalCoreListFilter object.
> Since use_first_core was only ever activated when logical core 0 was
> explicitly defined, core 0 can be removed from the list of total logical
> cores assuming that it was not listed within filter_specifier.
>
> This patch also removes 'vdevs' from 'system_under_test_node' and moves
> it into 'executions.'
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
<snip>
> diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py
> index 9e33b2825d..0c315a0da6 100644
> --- a/dts/framework/testbed_model/cpu.py
> +++ b/dts/framework/testbed_model/cpu.py
> @@ -210,6 +210,8 @@ def filter(self) -> list[LogicalCore]:
> Returns:
> The filtered cores.
> """
> + if 0 in self._lcores_to_filter:
> + self._lcores_to_filter = self._lcores_to_filter[1:]
> sockets_to_filter = self._filter_sockets(self._lcores_to_filter)
> filtered_lcores = []
> for socket_to_filter in sockets_to_filter:
> @@ -328,6 +330,9 @@ def filter(self) -> list[LogicalCore]:
> Return:
> The filtered logical CPU cores.
> """
> + if 0 not in self._filter_specifier.lcore_list:
> + self._lcores_to_filter = self._lcores_to_filter[1:]
> +
I don't really understand what these two conditionals are doing. if 0
is in the lcore_list, why do we need to omit the first value from the
filter list? Or if it is in the cores to filter why do we need to
remove the first element from that list? Also, is this attempting to
omit core 0 in the list? I think where it appears in the list can be
different depending on if the list is ascending or descending which is
different depending on the EAL parameters function it seems.
Regardless, can we add a comment on why this is needed?
> if not len(self._filter_specifier.lcore_list):
> return self._lcores_to_filter
>
<snip>
> 2.44.0
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 0/6] dts: Remove Excess Attributes From User Config
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (3 preceding siblings ...)
2024-06-13 20:18 ` [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-07-05 18:29 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
` (5 more replies)
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
` (6 subsequent siblings)
11 siblings, 6 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
v2:
* Patch series has been rebased to release candidate one.
* Added functionality to make 'test_suites' optional, based on certain
conditions.
* Aggregated all of the DPDK documentation into one holistic patch.
Nicholas Pratte (6):
dts: Remove build target config and list of devices
dts: Use First Core Logic Change
dts: Self-Discovering Architecture Change
dts: Rework DPDK Attributes In SUT Node Config
dts: add conditional behavior for test suite requirements
doc: dpdk documentation changes for new dts config
doc/guides/tools/dts.rst | 26 +---
dts/conf.yaml | 28 ++--
dts/framework/config/__init__.py | 83 +++++------
dts/framework/config/conf_yaml_schema.json | 142 +++----------------
dts/framework/config/types.py | 29 ++--
dts/framework/runner.py | 4 +-
dts/framework/test_result.py | 14 +-
dts/framework/testbed_model/cpu.py | 6 +-
dts/framework/testbed_model/linux_session.py | 5 +-
dts/framework/testbed_model/node.py | 21 +--
dts/framework/testbed_model/os_session.py | 10 +-
dts/framework/testbed_model/posix_session.py | 6 +
dts/framework/testbed_model/sut_node.py | 20 ++-
13 files changed, 130 insertions(+), 264 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
@ 2024-07-05 18:29 ` Nicholas Pratte
2024-11-06 19:29 ` Dean Marx
2024-07-05 18:31 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
` (4 subsequent siblings)
5 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:29 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
Remove the list of devices from the schema, as these are unuesed.
Likewise, removed build-target information since these is not currently
used, and it is unlikely to be used in the future. Adjustments to the
dts.rst are made to reflect these changes.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 5 +-
dts/framework/config/__init__.py | 30 +-------
dts/framework/config/conf_yaml_schema.json | 79 ----------------------
dts/framework/config/types.py | 6 --
dts/framework/runner.py | 2 +-
dts/framework/test_result.py | 14 +---
dts/framework/testbed_model/sut_node.py | 8 +--
7 files changed, 5 insertions(+), 139 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 7d95016e68..56cc08ced2 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -5,11 +5,8 @@
test_runs:
# define one test run environment
- build_targets:
- - arch: x86_64
- os: linux
- cpu: native
# the combination of the following two makes CC="ccache gcc"
- compiler: gcc
+ - compiler: gcc
compiler_wrapper: ccache
perf: false # disable performance testing
func: true # enable functional testing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..456a8a83ab 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -85,22 +85,6 @@ class OS(StrEnum):
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."""
@@ -341,28 +325,20 @@ class BuildTargetConfiguration:
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
compiler: Compiler
compiler_wrapper: str
- name: str
@classmethod
def from_dict(cls, d: BuildTargetConfigDict) -> Self:
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`.
+ `compiler` is converted to :class:`Enum`\s
Args:
d: The configuration dictionary.
@@ -371,12 +347,8 @@ def from_dict(cls, d: BuildTargetConfigDict) -> Self:
The build target configuration instance.
"""
return cls(
- arch=Architecture(d["arch"]),
- os=OS(d["os"]),
- cpu=CPUType(d["cpu"]),
compiler=Compiler(d["compiler"]),
compiler_wrapper=d.get("compiler_wrapper", ""),
- name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..3f7bc2acae 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,76 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "NIC": {
- "type": "string",
- "enum": [
- "ALL",
- "ConnectX3_MT4103",
- "ConnectX4_LX_MT4117",
- "ConnectX4_MT4115",
- "ConnectX5_MT4119",
- "ConnectX5_MT4121",
- "I40E_10G-10G_BASE_T_BC",
- "I40E_10G-10G_BASE_T_X722",
- "I40E_10G-SFP_X722",
- "I40E_10G-SFP_XL710",
- "I40E_10G-X722_A0",
- "I40E_1G-1G_BASE_T_X722",
- "I40E_25G-25G_SFP28",
- "I40E_40G-QSFP_A",
- "I40E_40G-QSFP_B",
- "IAVF-ADAPTIVE_VF",
- "IAVF-VF",
- "IAVF_10G-X722_VF",
- "ICE_100G-E810C_QSFP",
- "ICE_25G-E810C_SFP",
- "ICE_25G-E810_XXV_SFP",
- "IGB-I350_VF",
- "IGB_1G-82540EM",
- "IGB_1G-82545EM_COPPER",
- "IGB_1G-82571EB_COPPER",
- "IGB_1G-82574L",
- "IGB_1G-82576",
- "IGB_1G-82576_QUAD_COPPER",
- "IGB_1G-82576_QUAD_COPPER_ET2",
- "IGB_1G-82580_COPPER",
- "IGB_1G-I210_COPPER",
- "IGB_1G-I350_COPPER",
- "IGB_1G-I354_SGMII",
- "IGB_1G-PCH_LPTLP_I218_LM",
- "IGB_1G-PCH_LPTLP_I218_V",
- "IGB_1G-PCH_LPT_I217_LM",
- "IGB_1G-PCH_LPT_I217_V",
- "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
- "IGC-I225_LM",
- "IGC-I226_LM",
- "IXGBE_10G-82599_SFP",
- "IXGBE_10G-82599_SFP_SF_QP",
- "IXGBE_10G-82599_T3_LOM",
- "IXGBE_10G-82599_VF",
- "IXGBE_10G-X540T",
- "IXGBE_10G-X540_VF",
- "IXGBE_10G-X550EM_A_SFP",
- "IXGBE_10G-X550EM_X_10G_T",
- "IXGBE_10G-X550EM_X_SFP",
- "IXGBE_10G-X550EM_X_VF",
- "IXGBE_10G-X550T",
- "IXGBE_10G-X550_VF",
- "brcm_57414",
- "brcm_P2100G",
- "cavium_0011",
- "cavium_a034",
- "cavium_a063",
- "cavium_a064",
- "fastlinq_ql41000",
- "fastlinq_ql41000_vf",
- "fastlinq_ql45000",
- "fastlinq_ql45000_vf",
- "hi1822",
- "virtio"
- ]
- },
-
"ARCH": {
"type": "string",
"enum": [
@@ -124,12 +54,6 @@
"other"
]
},
- "os": {
- "$ref": "#/definitions/OS"
- },
- "cpu": {
- "$ref": "#/definitions/cpu"
- },
"compiler": {
"$ref": "#/definitions/compiler"
},
@@ -140,9 +64,6 @@
},
"additionalProperties": false,
"required": [
- "arch",
- "os",
- "cpu",
"compiler"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index cf16556403..2f75724c5e 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -74,12 +74,6 @@ class NodeConfigDict(TypedDict):
class BuildTargetConfigDict(TypedDict):
"""Allowed keys and values."""
- #:
- arch: str
- #:
- os: str
- #:
- cpu: str
#:
compiler: str
#:
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..2a1019899a 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -480,7 +480,7 @@ def _run_build_target(
test_suites_with_cases: The test suites with test cases to run.
"""
self._logger.set_stage(DtsStage.build_target_setup)
- self._logger.info(f"Running build target '{build_target_config.name}'.")
+ self._logger.info("Running build target.")
try:
sut_node.set_up_build_target(build_target_config)
diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index 5694a2482b..7fcc24fecd 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -31,12 +31,9 @@
from typing import Union
from .config import (
- OS,
- Architecture,
BuildTargetConfiguration,
BuildTargetInfo,
Compiler,
- CPUType,
NodeInfo,
TestRunConfiguration,
TestSuiteConfig,
@@ -223,7 +220,7 @@ class DTSResult(BaseResult):
"""Stores environment information and test results from a DTS run.
* Test run level information, such as testbed and the test suite list,
- * Build target level information, such as compiler, target OS and cpu,
+ * Build target level compiler information
* Test suite and test case results,
* All errors that are caught and recorded during DTS execution.
@@ -405,17 +402,11 @@ class BuildTargetResult(BaseResult):
The internal list stores the results of all test suites in a given build target.
Attributes:
- arch: The DPDK build target architecture.
- os: The DPDK build target operating system.
- cpu: The DPDK build target CPU.
compiler: The DPDK build target compiler.
compiler_version: The DPDK build target compiler version.
dpdk_version: The built DPDK version.
"""
- arch: Architecture
- os: OS
- cpu: CPUType
compiler: Compiler
compiler_version: str | None
dpdk_version: str | None
@@ -433,9 +424,6 @@ def __init__(
build_target_config: The build target's test run configuration.
"""
super().__init__()
- self.arch = build_target_config.arch
- self.os = build_target_config.os
- self.cpu = build_target_config.cpu
self.compiler = build_target_config.compiler
self.compiler_version = None
self.dpdk_version = None
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 2855fe0276..a4511157b7 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -115,12 +115,7 @@ def remote_dpdk_build_dir(self) -> PurePath:
This is the directory where DPDK was built.
We assume it was built in a subdirectory of the extracted tarball.
"""
- if self._build_target_config:
- return self.main_session.join_remote_path(
- self._remote_dpdk_dir, self._build_target_config.name
- )
- else:
- return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
+ return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
@property
def dpdk_version(self) -> str:
@@ -217,7 +212,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
"""Populate common environment variables and set build target config."""
self._env_vars = {}
self._build_target_config = build_target_config
- self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
self._env_vars["CC"] = build_target_config.compiler.name
if build_target_config.compiler_wrapper:
self._env_vars["CC"] = (
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-07-05 18:29 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
@ 2024-11-06 19:29 ` Dean Marx
0 siblings, 0 replies; 46+ messages in thread
From: Dean Marx @ 2024-11-06 19:29 UTC (permalink / raw)
To: Nicholas Pratte
Cc: paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli, dev
[-- Attachment #1: Type: text/plain, Size: 473 bytes --]
On Fri, Jul 5, 2024 at 2:29 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> Remove the list of devices from the schema, as these are unuesed.
> Likewise, removed build-target information since these is not currently
> used, and it is unlikely to be used in the future. Adjustments to the
> dts.rst are made to reflect these changes.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 871 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 2/6] dts: Use First Core Logic Change
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-07-05 18:29 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
@ 2024-07-05 18:31 ` Nicholas Pratte
2024-11-06 19:48 ` Dean Marx
2024-07-05 18:32 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
` (3 subsequent siblings)
5 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:31 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
Removed use_first_core from the conf.yaml in favor of determining this
within the framework. use_first_core continue to serve a purpose in that
it is only enabled when core 0 is explicitly provided in the
configuration. Any other configuration, including "" or "any," will
omit core 0.
Documentation reworks are included to reflect the changes made.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 3 +--
dts/framework/config/__init__.py | 11 +++++++----
dts/framework/config/conf_yaml_schema.json | 6 +-----
dts/framework/testbed_model/node.py | 9 +++++++++
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 56cc08ced2..53192e0761 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -29,8 +29,7 @@ nodes:
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
+ lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
hugepages_2mb: # optional; if removed, will use system hugepage configuration
number_of: 256
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 456a8a83ab..4c05373ef3 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -246,6 +246,9 @@ def from_dict(
hugepage_config_dict["force_first_numa"] = False
hugepage_config = HugepageConfiguration(**hugepage_config_dict)
+ lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
+ use_first_core = "0" in lcores
+
# The calls here contain duplicated code which is here because Mypy doesn't
# properly support dictionary unpacking with TypedDicts
if "traffic_generator" in d:
@@ -256,8 +259,8 @@ def from_dict(
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),
+ lcores=lcores,
+ use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
@@ -270,8 +273,8 @@ def from_dict(
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),
+ lcores=lcores,
+ use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
memory_channels=d.get("memory_channels", 1),
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 3f7bc2acae..01a6afdc72 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -163,13 +163,9 @@
},
"lcores": {
"type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
"description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
},
- "use_first_core": {
- "type": "boolean",
- "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
- },
"memory_channels": {
"type": "integer",
"description": "How many memory channels to use. Optional, defaults to 1."
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 12a40170ac..9b3f01f1e9 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -86,6 +86,15 @@ def __init__(self, node_config: NodeConfiguration):
self.lcores, LogicalCoreList(self.config.lcores)
).filter()
+ if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
+ self._logger.info(
+ """
+ WARNING: First core being used;
+ using the first core is considered risky and should only
+ be done by advanced users.
+ """
+ )
+
self._other_sessions = []
self._init_ports()
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 2/6] dts: Use First Core Logic Change
2024-07-05 18:31 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
@ 2024-11-06 19:48 ` Dean Marx
0 siblings, 0 replies; 46+ messages in thread
From: Dean Marx @ 2024-11-06 19:48 UTC (permalink / raw)
To: Nicholas Pratte
Cc: paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli, dev
[-- Attachment #1: Type: text/plain, Size: 581 bytes --]
On Fri, Jul 5, 2024 at 2:31 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> Removed use_first_core from the conf.yaml in favor of determining this
> within the framework. use_first_core continue to serve a purpose in that
> it is only enabled when core 0 is explicitly provided in the
> configuration. Any other configuration, including "" or "any," will
> omit core 0.
>
> Documentation reworks are included to reflect the changes made.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 1006 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 3/6] dts: Self-Discovering Architecture Change
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-07-05 18:29 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-07-05 18:31 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
@ 2024-07-05 18:32 ` Nicholas Pratte
2024-11-06 20:13 ` Dean Marx
2024-07-05 18:32 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
` (2 subsequent siblings)
5 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:32 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
The 'arch' attribute in the conf.yaml is unnecessary, as this can be
readily discovered within the constructor of any given node. Since OS is
determined within user configuration, finding system arch can be done
both reliably and easily within the framework.
For Linux/Posix systems, the 'uname' command is used to determine system
architecture. I believe that this is posix-standard and utilizes a
standardized output.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 2 --
dts/framework/config/__init__.py | 4 ----
dts/framework/config/conf_yaml_schema.json | 12 ------------
dts/framework/config/types.py | 2 --
dts/framework/testbed_model/node.py | 4 +++-
dts/framework/testbed_model/os_session.py | 8 ++++++++
dts/framework/testbed_model/posix_session.py | 6 ++++++
7 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 53192e0761..7ca4c05b55 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -27,7 +27,6 @@ nodes:
- name: "SUT 1"
hostname: sut1.change.me.localhost
user: dtsuser
- arch: x86_64
os: linux
lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
@@ -52,7 +51,6 @@ nodes:
- 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
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 4c05373ef3..662b3070a1 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -208,7 +208,6 @@ class NodeConfiguration:
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.
@@ -220,7 +219,6 @@ class NodeConfiguration:
hostname: str
user: str
password: str | None
- arch: Architecture
os: OS
lcores: str
use_first_core: bool
@@ -257,7 +255,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
@@ -271,7 +268,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 01a6afdc72..e65ea45058 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,14 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "ARCH": {
- "type": "string",
- "enum": [
- "x86_64",
- "arm64",
- "ppc64le"
- ]
- },
"OS": {
"type": "string",
"enum": [
@@ -155,9 +147,6 @@
"type": "string",
"description": "The password to use on this node. Use only as a last resort. SSH keys are STRONGLY preferred."
},
- "arch": {
- "$ref": "#/definitions/ARCH"
- },
"os": {
"$ref": "#/definitions/OS"
},
@@ -233,7 +222,6 @@
"name",
"hostname",
"user",
- "arch",
"os"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index 2f75724c5e..9934fef503 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -56,8 +56,6 @@ class NodeConfigDict(TypedDict):
#:
password: str
#:
- arch: str
- #:
os: str
#:
lcores: str
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 9b3f01f1e9..09399f4823 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -17,7 +17,7 @@
from ipaddress import IPv4Interface, IPv6Interface
from typing import Any, Callable, Union
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import OS, Architecture, NodeConfiguration, TestRunConfiguration
from framework.exception import ConfigurationError
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
@@ -55,6 +55,7 @@ class Node(ABC):
main_session: OSSession
config: NodeConfiguration
name: str
+ arch: Architecture
lcores: list[LogicalCore]
ports: list[Port]
_logger: DTSLogger
@@ -77,6 +78,7 @@ def __init__(self, node_config: NodeConfiguration):
self.name = node_config.name
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
+ self.arch = Architecture(self.main_session.get_arch_info())
self._logger.info(f"Connected to node: {self.name}")
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..02277eee1f 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -342,6 +342,14 @@ def get_node_info(self) -> NodeInfo:
Node information.
"""
+ @abstractmethod
+ def get_arch_info(self) -> str:
+ """Discover CPU architecture of the remote host.
+
+ Returns:
+ Remote host CPU architecture.
+ """
+
@abstractmethod
def update_ports(self, ports: list[Port]) -> None:
"""Get additional information about ports from the operating system and update them.
diff --git a/dts/framework/testbed_model/posix_session.py b/dts/framework/testbed_model/posix_session.py
index d279bb8b53..91afca61ea 100644
--- a/dts/framework/testbed_model/posix_session.py
+++ b/dts/framework/testbed_model/posix_session.py
@@ -295,3 +295,9 @@ def get_node_info(self) -> NodeInfo:
).stdout.split("\n")
kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
+
+ def get_arch_info(self) -> str:
+ """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
+ # return str(self.send_command('arch')).stdout
+
+ return str(self.send_command("uname -m").stdout.removesuffix("\n"))
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 3/6] dts: Self-Discovering Architecture Change
2024-07-05 18:32 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
@ 2024-11-06 20:13 ` Dean Marx
0 siblings, 0 replies; 46+ messages in thread
From: Dean Marx @ 2024-11-06 20:13 UTC (permalink / raw)
To: Nicholas Pratte
Cc: paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli, dev
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Fri, Jul 5, 2024 at 2:32 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> The 'arch' attribute in the conf.yaml is unnecessary, as this can be
> readily discovered within the constructor of any given node. Since OS is
> determined within user configuration, finding system arch can be done
> both reliably and easily within the framework.
>
> For Linux/Posix systems, the 'uname' command is used to determine system
> architecture. I believe that this is posix-standard and utilizes a
> standardized output.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 1077 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (2 preceding siblings ...)
2024-07-05 18:32 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
@ 2024-07-05 18:32 ` Nicholas Pratte
2024-11-06 20:32 ` Dean Marx
2024-07-05 18:33 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-05 18:33 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
5 siblings, 1 reply; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:32 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
Rework 'lcores' and 'memory_channels' into a new 'dpdk_config'
subsection in an effort to make these attributes SUT specific; the
traffic generator, more often than not, does not need this information.
Ideally, if such information is needed, then it will be listed in the
'traffic_generator' component in TG Node configuration. Such logic is
not introduced in this patch, but the framework can be rewritten to do
so without any implications of extreme effort.
To make this work, use_first_core has been removed from the framework
entirely in favor of doing this within the LogicalCoreListFilter object.
Since use_first_core was only ever activated when logical core 0 was
explicitly defined, core 0 can be removed from the list of total logical
cores assuming that it was not listed within filter_specifier.
This patch also removes 'vdevs' from 'system_under_test_node' and moves
it into 'executions.'
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 20 ++++-----
dts/framework/config/__init__.py | 45 +++++++++++--------
dts/framework/config/conf_yaml_schema.json | 47 ++++++++++----------
dts/framework/config/types.py | 21 ++++++---
dts/framework/testbed_model/cpu.py | 6 ++-
dts/framework/testbed_model/linux_session.py | 5 +--
dts/framework/testbed_model/node.py | 26 +----------
dts/framework/testbed_model/os_session.py | 2 +-
dts/framework/testbed_model/sut_node.py | 12 +++++
9 files changed, 95 insertions(+), 89 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 7ca4c05b55..bb1fbc86e3 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -14,12 +14,11 @@ test_runs:
test_suites: # the following test suites will be run in their entirety
- hello_world
- os_udp
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
# 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 test run
- - "crypto_openssl"
- # Traffic generator node to use for this test run
+ system_under_test_node: "SUT 1"
+ # 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
@@ -28,11 +27,6 @@ nodes:
hostname: sut1.change.me.localhost
user: dtsuser
os: linux
- lcores: "" # use all available logical cores (Skips first core)
- memory_channels: 4 # tells DPDK to use 4 memory channels
- hugepages_2mb: # optional; if removed, will use system hugepage configuration
- number_of: 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"
@@ -46,6 +40,12 @@ nodes:
os_driver: i40e
peer_node: "TG 1"
peer_pci: "0000:00:08.1"
+ hugepages_2mb: # optional; if removed, will use system hugepage configuration
+ number_of: 256
+ force_first_numa: false
+ dpdk_config:
+ lcores: "" # use all available logical cores (Skips first core)
+ memory_channels: 4 # tells DPDK to use 4 memory channels
# 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"
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 662b3070a1..ed1c979fb6 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -209,8 +209,6 @@ class NodeConfiguration:
password: The password of the user. The use of passwords is heavily discouraged.
Please use keys instead.
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.
"""
@@ -220,8 +218,6 @@ class NodeConfiguration:
user: str
password: str | None
os: OS
- lcores: str
- use_first_core: bool
hugepages: HugepageConfiguration | None
ports: list[PortConfig]
@@ -244,9 +240,6 @@ def from_dict(
hugepage_config_dict["force_first_numa"] = False
hugepage_config = HugepageConfiguration(**hugepage_config_dict)
- lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
- use_first_core = "0" in lcores
-
# The calls here contain duplicated code which is here because Mypy doesn't
# properly support dictionary unpacking with TypedDicts
if "traffic_generator" in d:
@@ -256,36 +249,54 @@ def from_dict(
user=d["user"],
password=d.get("password"),
os=OS(d["os"]),
- lcores=lcores,
- use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
)
else:
+ dpdk_config = d["dpdk_config"]
+ dpdk_config["lcores"] = (
+ "1"
+ if "lcores" not in dpdk_config
+ else dpdk_config["lcores"]
+ if "any" not in dpdk_config["lcores"]
+ else ""
+ )
+ dpdk_config["memory_channels"] = dpdk_config.get("memory_channels", 1)
return SutNodeConfiguration(
name=d["name"],
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
os=OS(d["os"]),
- lcores=lcores,
- use_first_core=use_first_core,
+ dpdk_config=DPDKConfig(**dpdk_config),
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 DPDKConfig:
+ """EAL parameters for executing and running DPDK.
+
+ Attributes:
+ lcores: Logical cores to be used for DPDK execution.
+ memory_channels: Memory channels to be used for DPDK execution.
+ """
+
+ lcores: str
+ memory_channels: int
+
+
@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.
+ dpdk_config: DPDK configuration attributes to be used during execution.
"""
- memory_channels: int
+ dpdk_config: DPDKConfig
@dataclass(slots=True, frozen=True)
@@ -450,7 +461,7 @@ def from_dict(
map(BuildTargetConfiguration.from_dict, d["build_targets"])
)
test_suites: list[TestSuiteConfig] = list(map(TestSuiteConfig.from_dict, d["test_suites"]))
- sut_name = d["system_under_test_node"]["node_name"]
+ sut_name = d["system_under_test_node"]
skip_smoke_tests = d.get("skip_smoke_tests", False)
assert sut_name in node_map, f"Unknown SUT {sut_name} in test run {d}"
system_under_test_node = node_map[sut_name]
@@ -465,9 +476,7 @@ def from_dict(
traffic_generator_node, TGNodeConfiguration
), f"Invalid TG configuration {traffic_generator_node}"
- vdevs = (
- d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
- )
+ vdevs = d["vdevs"] if "vdevs" in d else []
return cls(
build_targets=build_targets,
perf=d["perf"],
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index e65ea45058..b31f4d8dbe 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -150,14 +150,21 @@
"os": {
"$ref": "#/definitions/OS"
},
- "lcores": {
- "type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
- "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
- },
- "memory_channels": {
- "type": "integer",
- "description": "How many memory channels to use. Optional, defaults to 1."
+ "dpdk_config": {
+ "type": "object",
+ "description": "EAL arguments for DPDK execution",
+ "properties": {
+ "lcores": {
+ "type": "string",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
+ "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
+ },
+ "memory_channels": {
+ "type": "integer",
+ "description": "How many memory channels to use. Optional, defaults to 1."
+ }
+ },
+ "minimum": 1
},
"hugepages_2mb": {
"$ref": "#/definitions/hugepages_2mb"
@@ -264,23 +271,15 @@
"description": "Optional field that allows you to skip smoke testing",
"type": "boolean"
},
+ "vdevs": {
+ "description": "Optional list of names of vdevs to be used in execution",
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
"system_under_test_node": {
- "type":"object",
- "properties": {
- "node_name": {
- "$ref": "#/definitions/node_name"
- },
- "vdevs": {
- "description": "Optional list of names of vdevs to be used in the test run",
- "type": "array",
- "items": {
- "type": "string"
- }
- }
- },
- "required": [
- "node_name"
- ]
+ "$ref": "#/definitions/node_name"
},
"traffic_generator_node": {
"$ref": "#/definitions/node_name"
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index 9934fef503..004fc2b2d3 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -33,6 +33,15 @@ class TrafficGeneratorConfigDict(TypedDict):
type: str
+class DPDKConfigDict(TypedDict):
+ """Allowed keys and values."""
+
+ #:
+ memory_channels: int
+ #:
+ lcores: str
+
+
class HugepageConfigurationDict(TypedDict):
"""Allowed keys and values."""
@@ -58,15 +67,11 @@ class NodeConfigDict(TypedDict):
#:
os: str
#:
- lcores: str
- #:
- use_first_core: bool
- #:
ports: list[PortConfigDict]
#:
- memory_channels: int
- #:
traffic_generator: TrafficGeneratorConfigDict
+ #:
+ dpdk_config: DPDKConfigDict
class BuildTargetConfigDict(TypedDict):
@@ -110,9 +115,11 @@ class TestRunConfigDict(TypedDict):
#:
test_suites: TestSuiteConfigDict
#:
- system_under_test_node: TestRunSUTConfigDict
+ system_under_test_node: str
#:
traffic_generator_node: str
+ #:
+ vdevs: list[str]
class ConfigurationDict(TypedDict):
diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py
index a50cf44c19..cc4ca40ad9 100644
--- a/dts/framework/testbed_model/cpu.py
+++ b/dts/framework/testbed_model/cpu.py
@@ -167,7 +167,6 @@ def __init__(
# sorting by core is needed in case hyperthreading is enabled
self._lcores_to_filter = sorted(lcore_list, key=lambda x: x.core, reverse=not ascending)
- self.filter()
@abstractmethod
def filter(self) -> list[LogicalCore]:
@@ -210,6 +209,8 @@ def filter(self) -> list[LogicalCore]:
Returns:
The filtered cores.
"""
+ if 0 in self._lcores_to_filter:
+ self._lcores_to_filter = self._lcores_to_filter[1:]
sockets_to_filter = self._filter_sockets(self._lcores_to_filter)
filtered_lcores = []
for socket_to_filter in sockets_to_filter:
@@ -328,6 +329,9 @@ def filter(self) -> list[LogicalCore]:
Return:
The filtered logical CPU cores.
"""
+ if 0 not in self._filter_specifier.lcore_list:
+ self._lcores_to_filter = self._lcores_to_filter[1:]
+
if not len(self._filter_specifier.lcore_list):
return self._lcores_to_filter
diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 99abc21353..347d01878c 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -68,15 +68,12 @@ class LinuxSession(PosixSession):
def _get_privileged_command(command: str) -> str:
return f"sudo -- sh -c '{command}'"
- def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
+ def get_remote_cpus(self) -> list[LogicalCore]:
"""Overrides :meth:`~.os_session.OSSession.get_remote_cpus`."""
cpu_info = self.send_command("lscpu -p=CPU,CORE,SOCKET,NODE|grep -v \\#").stdout
lcores = []
for cpu_line in cpu_info.splitlines():
lcore, core, socket, node = map(int, cpu_line.split(","))
- if core == 0 and socket == 0 and not use_first_core:
- self._logger.info("Not using the first physical core.")
- continue
lcores.append(LogicalCore(lcore, core, socket, node))
return lcores
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 09399f4823..9630407247 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -22,13 +22,7 @@
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
-from .cpu import (
- LogicalCore,
- LogicalCoreCount,
- LogicalCoreList,
- LogicalCoreListFilter,
- lcore_filter,
-)
+from .cpu import LogicalCore, LogicalCoreCount, LogicalCoreList, lcore_filter
from .linux_session import LinuxSession
from .os_session import OSSession
from .port import Port
@@ -79,24 +73,8 @@ def __init__(self, node_config: NodeConfiguration):
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
self.arch = Architecture(self.main_session.get_arch_info())
-
self._logger.info(f"Connected to node: {self.name}")
-
self._get_remote_cpus()
- # filter the node lcores according to the test run configuration
- self.lcores = LogicalCoreListFilter(
- self.lcores, LogicalCoreList(self.config.lcores)
- ).filter()
-
- if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
- self._logger.info(
- """
- WARNING: First core being used;
- using the first core is considered risky and should only
- be done by advanced users.
- """
- )
-
self._other_sessions = []
self._init_ports()
@@ -182,7 +160,7 @@ def filter_lcores(
def _get_remote_cpus(self) -> None:
"""Scan CPUs in the remote OS and store a list of LogicalCores."""
self._logger.info("Getting CPU information.")
- self.lcores = self.main_session.get_remote_cpus(self.config.use_first_core)
+ self.lcores = self.main_session.get_remote_cpus()
def _setup_hugepages(self) -> None:
"""Setup hugepages on the node.
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 02277eee1f..f217a40e7f 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -280,7 +280,7 @@ def get_dpdk_version(self, version_path: str | PurePath) -> str:
"""
@abstractmethod
- def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
+ def get_remote_cpus(self) -> list[LogicalCore]:
r"""Get the list of :class:`~.cpu.LogicalCore`\s on the remote node.
Args:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index a4511157b7..178535b617 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -29,6 +29,7 @@
from framework.settings import SETTINGS
from framework.utils import MesonArgs
+from .cpu import LogicalCore, LogicalCoreList
from .node import Node
from .os_session import OSSession
from .virtual_device import VirtualDevice
@@ -75,6 +76,17 @@ def __init__(self, node_config: SutNodeConfiguration):
node_config: The SUT node's test run configuration.
"""
super().__init__(node_config)
+ self.lcores = self.filter_lcores(LogicalCoreList(self.config.dpdk_config.lcores))
+ if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
+ self._logger.info(
+ """
+ WARNING: First core being used;
+ using the first core is considered risky and should only
+ be done by advanced users.
+ """
+ )
+ else:
+ self._logger.info("Not using first core")
self.virtual_devices = []
self.dpdk_prefix_list = []
self._build_target_config = None
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config
2024-07-05 18:32 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
@ 2024-11-06 20:32 ` Dean Marx
0 siblings, 0 replies; 46+ messages in thread
From: Dean Marx @ 2024-11-06 20:32 UTC (permalink / raw)
To: Nicholas Pratte
Cc: paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli, dev
[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]
On Fri, Jul 5, 2024 at 2:32 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> Rework 'lcores' and 'memory_channels' into a new 'dpdk_config'
> subsection in an effort to make these attributes SUT specific; the
> traffic generator, more often than not, does not need this information.
> Ideally, if such information is needed, then it will be listed in the
> 'traffic_generator' component in TG Node configuration. Such logic is
> not introduced in this patch, but the framework can be rewritten to do
> so without any implications of extreme effort.
>
> To make this work, use_first_core has been removed from the framework
> entirely in favor of doing this within the LogicalCoreListFilter object.
> Since use_first_core was only ever activated when logical core 0 was
> explicitly defined, core 0 can be removed from the list of total logical
> cores assuming that it was not listed within filter_specifier.
>
> This patch also removes 'vdevs' from 'system_under_test_node' and moves
> it into 'executions.'
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
<snip>
> - memory_channels: 4 # tells DPDK to use 4 memory channels
> - hugepages_2mb: # optional; if removed, will use system hugepage
> configuration
> - number_of: 256
> - force_first_numa: false
>
Just curious, is there a reason 256 is the default value?
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 2098 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 5/6] dts: add conditional behavior for test suite
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (3 preceding siblings ...)
2024-07-05 18:32 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
@ 2024-07-05 18:33 ` Nicholas Pratte
2024-07-05 18:33 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
5 siblings, 0 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:33 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
There is some odd functionality/behavior in how the --test-suite
parameters interacts in conjunction with the 'test_suites' attribute in
the config file. If a user leaves an empty list underneath
'test_suites,' or if they negate the attribute entirely, even if said
user adds test suites via the --test-suite parameter, a schema violation
is thrown.
This patch mitigates this, by removing the schema requirement if the
user has indicated test suites within main.py parameters, allowing for
the 'test_suites' attribute to be optional.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/framework/config/__init__.py | 7 ++++++-
dts/framework/runner.py | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index ed1c979fb6..82182b5c99 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -553,7 +553,7 @@ def from_dict(cls, d: ConfigurationDict) -> Self:
return cls(test_runs=test_runs)
-def load_config(config_file_path: Path) -> Configuration:
+def load_config(config_file_path: Path, test_suites: list[TestSuiteConfig]) -> Configuration:
"""Load DTS test run configuration from a file.
Load the YAML test run configuration file
@@ -576,6 +576,11 @@ def load_config(config_file_path: Path) -> Configuration:
with open(schema_path, "r") as f:
schema = json.load(f)
+ if test_suites:
+ schema["properties"]["test_runs"]["items"]["required"].remove("test_suites")
+ for test_run in config_data["test_runs"]:
+ if not hasattr(test_run, "test_suites"):
+ test_run["test_suites"] = []
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/runner.py b/dts/framework/runner.py
index 2a1019899a..edda5510af 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -85,7 +85,7 @@ class DTSRunner:
def __init__(self):
"""Initialize the instance with configuration, logger, result and string constants."""
- self._configuration = load_config(SETTINGS.config_file_path)
+ self._configuration = load_config(SETTINGS.config_file_path, SETTINGS.test_suites)
self._logger = get_dts_logger()
if not os.path.exists(SETTINGS.output_dir):
os.makedirs(SETTINGS.output_dir)
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH v2 6/6] doc: dpdk documentation changes for new dts config
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (4 preceding siblings ...)
2024-07-05 18:33 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
@ 2024-07-05 18:33 ` Nicholas Pratte
5 siblings, 0 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:33 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
Adjusted DPDK documentation to reflect the changes made to the dts
conf.yaml configuration file.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
doc/guides/tools/dts.rst | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 515b15e4d8..a64580e0de 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -437,14 +437,6 @@ _`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``.
@@ -456,9 +448,6 @@ _`Build target`
*mapping* – 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.
@@ -565,18 +554,15 @@ involved in the testing. These can be defined with the following mappings:
| | |
| | **NB**: Use only as last resort. SSH keys are **strongly** preferred. |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``arch`` | The architecture of this node. See `ARCH`_ for supported values. |
- +-----------------------+---------------------------------------------------------------------------------------+
| ``os`` | The operating system of this node. See `OS`_ for supported values. |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``lcores`` | | (*optional*, defaults to 1) *string* – Comma-separated list of logical |
- | | | cores to use. An empty string means use all lcores. |
- | | |
- | | **Example**: ``1,2,3,4,5,18-22`` |
+ | ``dpdk_config`` | Configuration relating to DPDK (to be specified on SUT Nodes) |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``use_first_core`` | (*optional*, defaults to ``false``) *boolean* |
+ | ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
+ | | | cores to use. An empty string means use all lcores except core 0. core 0 is used |
+ | | | only when explicitly specified |
| | |
- | | Indicates whether DPDK should use only the first physical core or not. |
+ | | **Example**: ``1,2,3,4,5,18-22`` |
+-----------------------+---------------------------------------------------------------------------------------+
| ``memory_channels`` | (*optional*, defaults to 1) *integer* |
| | |
@@ -617,4 +603,4 @@ And they both have two network ports which are physically connected to each othe
.. literalinclude:: ../../../dts/conf.yaml
:language: yaml
- :start-at: test_runs:
+ :start-at: test_runs:
\ No newline at end of file
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (4 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-07-16 15:07 ` Jeremy Spewock
` (2 more replies)
2024-07-05 17:13 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
` (5 subsequent siblings)
11 siblings, 3 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
Remove the list of devices from the schema, as these are unuesed.
Likewise, removed build-target information since these is not currently
used, and it is unlikely to be used in the future. Adjustments to the
dts.rst are made to reflect these changes.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 5 +-
dts/framework/config/__init__.py | 30 +-------
dts/framework/config/conf_yaml_schema.json | 79 ----------------------
dts/framework/config/types.py | 6 --
dts/framework/runner.py | 2 +-
dts/framework/test_result.py | 14 +---
dts/framework/testbed_model/sut_node.py | 8 +--
7 files changed, 5 insertions(+), 139 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 7d95016e68..56cc08ced2 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -5,11 +5,8 @@
test_runs:
# define one test run environment
- build_targets:
- - arch: x86_64
- os: linux
- cpu: native
# the combination of the following two makes CC="ccache gcc"
- compiler: gcc
+ - compiler: gcc
compiler_wrapper: ccache
perf: false # disable performance testing
func: true # enable functional testing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..456a8a83ab 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -85,22 +85,6 @@ class OS(StrEnum):
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."""
@@ -341,28 +325,20 @@ class BuildTargetConfiguration:
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
compiler: Compiler
compiler_wrapper: str
- name: str
@classmethod
def from_dict(cls, d: BuildTargetConfigDict) -> Self:
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`.
+ `compiler` is converted to :class:`Enum`\s
Args:
d: The configuration dictionary.
@@ -371,12 +347,8 @@ def from_dict(cls, d: BuildTargetConfigDict) -> Self:
The build target configuration instance.
"""
return cls(
- arch=Architecture(d["arch"]),
- os=OS(d["os"]),
- cpu=CPUType(d["cpu"]),
compiler=Compiler(d["compiler"]),
compiler_wrapper=d.get("compiler_wrapper", ""),
- name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..3f7bc2acae 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,76 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "NIC": {
- "type": "string",
- "enum": [
- "ALL",
- "ConnectX3_MT4103",
- "ConnectX4_LX_MT4117",
- "ConnectX4_MT4115",
- "ConnectX5_MT4119",
- "ConnectX5_MT4121",
- "I40E_10G-10G_BASE_T_BC",
- "I40E_10G-10G_BASE_T_X722",
- "I40E_10G-SFP_X722",
- "I40E_10G-SFP_XL710",
- "I40E_10G-X722_A0",
- "I40E_1G-1G_BASE_T_X722",
- "I40E_25G-25G_SFP28",
- "I40E_40G-QSFP_A",
- "I40E_40G-QSFP_B",
- "IAVF-ADAPTIVE_VF",
- "IAVF-VF",
- "IAVF_10G-X722_VF",
- "ICE_100G-E810C_QSFP",
- "ICE_25G-E810C_SFP",
- "ICE_25G-E810_XXV_SFP",
- "IGB-I350_VF",
- "IGB_1G-82540EM",
- "IGB_1G-82545EM_COPPER",
- "IGB_1G-82571EB_COPPER",
- "IGB_1G-82574L",
- "IGB_1G-82576",
- "IGB_1G-82576_QUAD_COPPER",
- "IGB_1G-82576_QUAD_COPPER_ET2",
- "IGB_1G-82580_COPPER",
- "IGB_1G-I210_COPPER",
- "IGB_1G-I350_COPPER",
- "IGB_1G-I354_SGMII",
- "IGB_1G-PCH_LPTLP_I218_LM",
- "IGB_1G-PCH_LPTLP_I218_V",
- "IGB_1G-PCH_LPT_I217_LM",
- "IGB_1G-PCH_LPT_I217_V",
- "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
- "IGC-I225_LM",
- "IGC-I226_LM",
- "IXGBE_10G-82599_SFP",
- "IXGBE_10G-82599_SFP_SF_QP",
- "IXGBE_10G-82599_T3_LOM",
- "IXGBE_10G-82599_VF",
- "IXGBE_10G-X540T",
- "IXGBE_10G-X540_VF",
- "IXGBE_10G-X550EM_A_SFP",
- "IXGBE_10G-X550EM_X_10G_T",
- "IXGBE_10G-X550EM_X_SFP",
- "IXGBE_10G-X550EM_X_VF",
- "IXGBE_10G-X550T",
- "IXGBE_10G-X550_VF",
- "brcm_57414",
- "brcm_P2100G",
- "cavium_0011",
- "cavium_a034",
- "cavium_a063",
- "cavium_a064",
- "fastlinq_ql41000",
- "fastlinq_ql41000_vf",
- "fastlinq_ql45000",
- "fastlinq_ql45000_vf",
- "hi1822",
- "virtio"
- ]
- },
-
"ARCH": {
"type": "string",
"enum": [
@@ -124,12 +54,6 @@
"other"
]
},
- "os": {
- "$ref": "#/definitions/OS"
- },
- "cpu": {
- "$ref": "#/definitions/cpu"
- },
"compiler": {
"$ref": "#/definitions/compiler"
},
@@ -140,9 +64,6 @@
},
"additionalProperties": false,
"required": [
- "arch",
- "os",
- "cpu",
"compiler"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index cf16556403..2f75724c5e 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -74,12 +74,6 @@ class NodeConfigDict(TypedDict):
class BuildTargetConfigDict(TypedDict):
"""Allowed keys and values."""
- #:
- arch: str
- #:
- os: str
- #:
- cpu: str
#:
compiler: str
#:
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..2a1019899a 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -480,7 +480,7 @@ def _run_build_target(
test_suites_with_cases: The test suites with test cases to run.
"""
self._logger.set_stage(DtsStage.build_target_setup)
- self._logger.info(f"Running build target '{build_target_config.name}'.")
+ self._logger.info("Running build target.")
try:
sut_node.set_up_build_target(build_target_config)
diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index 5694a2482b..7fcc24fecd 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -31,12 +31,9 @@
from typing import Union
from .config import (
- OS,
- Architecture,
BuildTargetConfiguration,
BuildTargetInfo,
Compiler,
- CPUType,
NodeInfo,
TestRunConfiguration,
TestSuiteConfig,
@@ -223,7 +220,7 @@ class DTSResult(BaseResult):
"""Stores environment information and test results from a DTS run.
* Test run level information, such as testbed and the test suite list,
- * Build target level information, such as compiler, target OS and cpu,
+ * Build target level compiler information
* Test suite and test case results,
* All errors that are caught and recorded during DTS execution.
@@ -405,17 +402,11 @@ class BuildTargetResult(BaseResult):
The internal list stores the results of all test suites in a given build target.
Attributes:
- arch: The DPDK build target architecture.
- os: The DPDK build target operating system.
- cpu: The DPDK build target CPU.
compiler: The DPDK build target compiler.
compiler_version: The DPDK build target compiler version.
dpdk_version: The built DPDK version.
"""
- arch: Architecture
- os: OS
- cpu: CPUType
compiler: Compiler
compiler_version: str | None
dpdk_version: str | None
@@ -433,9 +424,6 @@ def __init__(
build_target_config: The build target's test run configuration.
"""
super().__init__()
- self.arch = build_target_config.arch
- self.os = build_target_config.os
- self.cpu = build_target_config.cpu
self.compiler = build_target_config.compiler
self.compiler_version = None
self.dpdk_version = None
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 2855fe0276..a4511157b7 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -115,12 +115,7 @@ def remote_dpdk_build_dir(self) -> PurePath:
This is the directory where DPDK was built.
We assume it was built in a subdirectory of the extracted tarball.
"""
- if self._build_target_config:
- return self.main_session.join_remote_path(
- self._remote_dpdk_dir, self._build_target_config.name
- )
- else:
- return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
+ return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
@property
def dpdk_version(self) -> str:
@@ -217,7 +212,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
"""Populate common environment variables and set build target config."""
self._env_vars = {}
self._build_target_config = build_target_config
- self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
self._env_vars["CC"] = build_target_config.compiler.name
if build_target_config.compiler_wrapper:
self._env_vars["CC"] = (
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
@ 2024-07-16 15:07 ` Jeremy Spewock
2024-09-12 20:33 ` Nicholas Pratte
2024-09-10 11:30 ` Juraj Linkeš
2024-11-18 16:51 ` Luca Vizzarro
2 siblings, 1 reply; 46+ messages in thread
From: Jeremy Spewock @ 2024-07-16 15:07 UTC (permalink / raw)
To: Nicholas Pratte
Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
paul.szczepanek, juraj.linkes, dev
On Fri, Jul 5, 2024 at 1:15 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Remove the list of devices from the schema, as these are unuesed.
> Likewise, removed build-target information since these is not currently
> used, and it is unlikely to be used in the future. Adjustments to the
> dts.rst are made to reflect these changes.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
<snip>
> @unique
> class Compiler(StrEnum):
> r"""The supported compilers of :class:`~framework.testbed_model.node.Node`\s."""
> @@ -341,28 +325,20 @@ class BuildTargetConfiguration:
> 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
> compiler: Compiler
> compiler_wrapper: str
> - name: str
>
> @classmethod
> def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> 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`.
> + `compiler` is converted to :class:`Enum`\s
Because it's only the one attribute now this should likely just be
"`compiler` is converted to an :class:`Enum`" and because you don't
need this \s, I think you can also remove the "r" from the start of
the doc-string that makes it a string literal.
>
> Args:
> d: The configuration dictionary.
> @@ -371,12 +347,8 @@ def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> The build target configuration instance.
> """
> return cls(
> - arch=Architecture(d["arch"]),
> - os=OS(d["os"]),
> - cpu=CPUType(d["cpu"]),
> compiler=Compiler(d["compiler"]),
> compiler_wrapper=d.get("compiler_wrapper", ""),
> - name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
> )
>
>
<snip>
>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-07-16 15:07 ` Jeremy Spewock
@ 2024-09-12 20:33 ` Nicholas Pratte
0 siblings, 0 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-09-12 20:33 UTC (permalink / raw)
To: Jeremy Spewock
Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
paul.szczepanek, juraj.linkes, dev
<snip>
> > @classmethod
> > def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> > 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`.
> > + `compiler` is converted to :class:`Enum`\s
>
> Because it's only the one attribute now this should likely just be
> "`compiler` is converted to an :class:`Enum`" and because you don't
> need this \s, I think you can also remove the "r" from the start of
> the doc-string that makes it a string literal.
I could have sworn I fixed this before sending this out. I'll take care of it!
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-07-16 15:07 ` Jeremy Spewock
@ 2024-09-10 11:30 ` Juraj Linkeš
2024-09-12 20:31 ` Nicholas Pratte
2024-11-18 16:51 ` Luca Vizzarro
2 siblings, 1 reply; 46+ messages in thread
From: Juraj Linkeš @ 2024-09-10 11:30 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, luca.vizzarro,
yoan.picchi, Honnappa.Nagarahalli, paul.szczepanek
Cc: dev
The subject line should be all lowercase (except for abbreviations and
maybe some other exceptions).
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> Remove the list of devices from the schema, as these are unuesed.
Typo: unuesed
> Likewise, removed build-target information since these is not currently
> used, and it is unlikely to be used in the future. Adjustments to the
> dts.rst are made to reflect these changes.
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> @@ -341,28 +325,20 @@ class BuildTargetConfiguration:
> 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
> compiler: Compiler
> compiler_wrapper: str
> - name: str
>
> @classmethod
> def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> 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`.
> + `compiler` is converted to :class:`Enum`\s
>
> Args:
> d: The configuration dictionary.
> @@ -371,12 +347,8 @@ def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> The build target configuration instance.
> """
> return cls(
> - arch=Architecture(d["arch"]),
> - os=OS(d["os"]),
> - cpu=CPUType(d["cpu"]),
> compiler=Compiler(d["compiler"]),
> compiler_wrapper=d.get("compiler_wrapper", ""),
> - name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
I think we can still keep this, we can just remove arch, os and cpu
(there could be multiple build target, each with different compiler
(gcc, clang, etc.)). If there's a reason to remove this, it should be
mentioned in the commit message.
> )
>
>
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index f02a310bb5..3f7bc2acae 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -6,76 +6,6 @@
> "ARCH": {
> "type": "string",
> "enum": [
> @@ -124,12 +54,6 @@
> "other"
> ]
> },
> - "os": {
> - "$ref": "#/definitions/OS"
> - },
> - "cpu": {
> - "$ref": "#/definitions/cpu"
> - },
Should we also remove arch that's above these two?
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 6b6f6a05f5..2a1019899a 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -480,7 +480,7 @@ def _run_build_target(
> test_suites_with_cases: The test suites with test cases to run.
> """
> self._logger.set_stage(DtsStage.build_target_setup)
> - self._logger.info(f"Running build target '{build_target_config.name}'.")
> + self._logger.info("Running build target.")
If we keep build_target_config.name this should be reverted.
>
> try:
> sut_node.set_up_build_target(build_target_config)
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index 2855fe0276..a4511157b7 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -115,12 +115,7 @@ def remote_dpdk_build_dir(self) -> PurePath:
> This is the directory where DPDK was built.
> We assume it was built in a subdirectory of the extracted tarball.
> """
> - if self._build_target_config:
> - return self.main_session.join_remote_path(
> - self._remote_dpdk_dir, self._build_target_config.name
> - )
> - else:
> - return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
> + return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
>
Same here, revert if we keep build_target_config.name.
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-09-10 11:30 ` Juraj Linkeš
@ 2024-09-12 20:31 ` Nicholas Pratte
0 siblings, 0 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-09-12 20:31 UTC (permalink / raw)
To: Juraj Linkeš
Cc: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, dev
On Tue, Sep 10, 2024 at 7:30 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> The subject line should be all lowercase (except for abbreviations and
> maybe some other exceptions).
Ack.
>
> On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> > Remove the list of devices from the schema, as these are unuesed.
>
> Typo: unused
Thanks for pointing this out!
>
> > Likewise, removed build-target information since these is not currently
> > used, and it is unlikely to be used in the future. Adjustments to the
> > dts.rst are made to reflect these changes.
>
> > diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
>
> > @@ -341,28 +325,20 @@ class BuildTargetConfiguration:
> > 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
> > compiler: Compiler
> > compiler_wrapper: str
> > - name: str
> >
> > @classmethod
> > def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> > 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`.
> > + `compiler` is converted to :class:`Enum`\s
> >
> > Args:
> > d: The configuration dictionary.
> > @@ -371,12 +347,8 @@ def from_dict(cls, d: BuildTargetConfigDict) -> Self:
> > The build target configuration instance.
> > """
> > return cls(
> > - arch=Architecture(d["arch"]),
> > - os=OS(d["os"]),
> > - cpu=CPUType(d["cpu"]),
> > compiler=Compiler(d["compiler"]),
> > compiler_wrapper=d.get("compiler_wrapper", ""),
> > - name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
>
> I think we can still keep this, we can just remove arch, os and cpu
> (there could be multiple build target, each with different compiler
> (gcc, clang, etc.)). If there's a reason to remove this, it should be
> mentioned in the commit message.
I can do that. If I had a justification for removing this way back
when I wrote this, it wasn't strong enough for me to remember. I don't
mind leaving this in.
>
> > )
> >
> >
> > diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> > index f02a310bb5..3f7bc2acae 100644
> > --- a/dts/framework/config/conf_yaml_schema.json
> > +++ b/dts/framework/config/conf_yaml_schema.json
> > @@ -6,76 +6,6 @@
>
> > "ARCH": {
> > "type": "string",
> > "enum": [
> > @@ -124,12 +54,6 @@
> > "other"
> > ]
> > },
> > - "os": {
> > - "$ref": "#/definitions/OS"
> > - },
> > - "cpu": {
> > - "$ref": "#/definitions/cpu"
> > - },
>
> Should we also remove arch that's above these two?
Must have missed that! There's no reason for that to be there now that
it's being removed.
>
>
> > diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> > index 6b6f6a05f5..2a1019899a 100644
> > --- a/dts/framework/runner.py
> > +++ b/dts/framework/runner.py
> > @@ -480,7 +480,7 @@ def _run_build_target(
> > test_suites_with_cases: The test suites with test cases to run.
> > """
> > self._logger.set_stage(DtsStage.build_target_setup)
> > - self._logger.info(f"Running build target '{build_target_config.name}'.")
> > + self._logger.info("Running build target.")
>
> If we keep build_target_config.name this should be reverted.
Ack.
>
> >
> > try:
> > sut_node.set_up_build_target(build_target_config)
>
> > diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> > index 2855fe0276..a4511157b7 100644
> > --- a/dts/framework/testbed_model/sut_node.py
> > +++ b/dts/framework/testbed_model/sut_node.py
> > @@ -115,12 +115,7 @@ def remote_dpdk_build_dir(self) -> PurePath:
> > This is the directory where DPDK was built.
> > We assume it was built in a subdirectory of the extracted tarball.
> > """
> > - if self._build_target_config:
> > - return self.main_session.join_remote_path(
> > - self._remote_dpdk_dir, self._build_target_config.name
> > - )
> > - else:
> > - return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
> > + return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
> >
>
> Same here, revert if we keep build_target_config.name.
Ack.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-07-16 15:07 ` Jeremy Spewock
2024-09-10 11:30 ` Juraj Linkeš
@ 2024-11-18 16:51 ` Luca Vizzarro
2 siblings, 0 replies; 46+ messages in thread
From: Luca Vizzarro @ 2024-11-18 16:51 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev
Hi Nick,
I second the comments already made. A couple of nits on top of those:
On 05/07/2024 18:13, Nicholas Pratte wrote:
> Remove the list of devices from the schema, as these are unuesed.
> Likewise, removed build-target information since these is not currently
> used, and it is unlikely to be used in the future. Adjustments to the
> dts.rst are made to reflect these changes.
There are no adjustments made to the dts.rst here, so it shouldn't be
mentioned.
>
> Bugzilla ID: 1360
There should be an empty line between Signed-off-by and the metatags.
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
<snip>
> @unique
> class Compiler(StrEnum):
> r"""The supported compilers of :class:`~framework.testbed_model.node.Node`\s."""
> @@ -341,28 +325,20 @@ class BuildTargetConfiguration:
> 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.
I am a bit confused whether you are removing `name` or not, but if you
are you forgot to remove this entry from the docstring.
> """
>
> - arch: Architecture
> - os: OS
> - cpu: CPUType
> compiler: Compiler
> compiler_wrapper: str
> - name: str
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 2/6] dts: Use First Core Logic Change
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (5 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-09-10 13:34 ` Juraj Linkeš
2024-11-18 16:54 ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
` (4 subsequent siblings)
11 siblings, 2 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
Removed use_first_core from the conf.yaml in favor of determining this
within the framework. use_first_core continue to serve a purpose in that
it is only enabled when core 0 is explicitly provided in the
configuration. Any other configuration, including "" or "any," will
omit core 0.
Documentation reworks are included to reflect the changes made.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 3 +--
dts/framework/config/__init__.py | 11 +++++++----
dts/framework/config/conf_yaml_schema.json | 6 +-----
dts/framework/testbed_model/node.py | 9 +++++++++
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 56cc08ced2..53192e0761 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -29,8 +29,7 @@ nodes:
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
+ lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
hugepages_2mb: # optional; if removed, will use system hugepage configuration
number_of: 256
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 456a8a83ab..4c05373ef3 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -246,6 +246,9 @@ def from_dict(
hugepage_config_dict["force_first_numa"] = False
hugepage_config = HugepageConfiguration(**hugepage_config_dict)
+ lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
+ use_first_core = "0" in lcores
+
# The calls here contain duplicated code which is here because Mypy doesn't
# properly support dictionary unpacking with TypedDicts
if "traffic_generator" in d:
@@ -256,8 +259,8 @@ def from_dict(
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),
+ lcores=lcores,
+ use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
@@ -270,8 +273,8 @@ def from_dict(
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),
+ lcores=lcores,
+ use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
memory_channels=d.get("memory_channels", 1),
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 3f7bc2acae..01a6afdc72 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -163,13 +163,9 @@
},
"lcores": {
"type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
"description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
},
- "use_first_core": {
- "type": "boolean",
- "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
- },
"memory_channels": {
"type": "integer",
"description": "How many memory channels to use. Optional, defaults to 1."
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 12a40170ac..9b3f01f1e9 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -86,6 +86,15 @@ def __init__(self, node_config: NodeConfiguration):
self.lcores, LogicalCoreList(self.config.lcores)
).filter()
+ if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
+ self._logger.info(
+ """
+ WARNING: First core being used;
+ using the first core is considered risky and should only
+ be done by advanced users.
+ """
+ )
+
self._other_sessions = []
self._init_ports()
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 2/6] dts: Use First Core Logic Change
2024-07-05 17:13 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
@ 2024-09-10 13:34 ` Juraj Linkeš
2024-11-18 16:54 ` Luca Vizzarro
1 sibling, 0 replies; 46+ messages in thread
From: Juraj Linkeš @ 2024-09-10 13:34 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, luca.vizzarro,
yoan.picchi, Honnappa.Nagarahalli, paul.szczepanek
Cc: dev
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> Removed use_first_core from the conf.yaml in favor of determining this
> within the framework. use_first_core continue to serve a purpose in that
> it is only enabled when core 0 is explicitly provided in the
> configuration. Any other configuration, including "" or "any," will
> omit core 0.
>
> Documentation reworks are included to reflect the changes made.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
> dts/conf.yaml | 3 +--
> dts/framework/config/__init__.py | 11 +++++++----
> dts/framework/config/conf_yaml_schema.json | 6 +-----
> dts/framework/testbed_model/node.py | 9 +++++++++
> 4 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 56cc08ced2..53192e0761 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -29,8 +29,7 @@ nodes:
> 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
> + lcores: "" # use all available logical cores (Skips first core)
The message in parentheses could be confusing, let's make it explicit
that an empty string skips first core.
And the comments in this file are all lowercase so let's do that in
parentheses as well.
> memory_channels: 4 # tells DPDK to use 4 memory channels
> hugepages_2mb: # optional; if removed, will use system hugepage configuration
> number_of: 256
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index 456a8a83ab..4c05373ef3 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -246,6 +246,9 @@ def from_dict(
> hugepage_config_dict["force_first_numa"] = False
> hugepage_config = HugepageConfiguration(**hugepage_config_dict)
>
> + lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
> + use_first_core = "0" in lcores
> +
I'm wondering whether we want to do this in config. The logic seems to
be better placed elsewhere. My thinking is this:
1. Removing use_first_core from Node._get_remote_cpus() (and the
downstream OSSession methods) makes sense since the method really looks
like it should be getting all remote CPUs. A benefit here is the first
core filtering doesn't happen in OSSession where it currently is - it
just doesn't make sense there.
2. Putting use_first_core to LogicalCoreList also doesn't make much
sense, it's supposed to be a list and not supposed to do any filtering.
3. The two above are used to compose the final list of usable cores on a
node. We could either filter the first core from 1. after getting the
cores, we could filter it from 2. after getting the core list or filter
it after using 1. and 2. with LogicalCoreListFilter().
Or in other words:
remote_lcores = self.main_session.get_remote_cpus()
lcore_list_config = LogicalCoreList(self.config.lcores)
self.lcores = LogicalCoreListFilter(remote_lcores,
lcore_list_config).filter()
We can either remove the first core from remote_lcores,
lcore_list_config or from self.lcores. The result is the same if we
remove it from any of these, so maybe we just work with self.lcores as
that is also what you've used in the patch to issue the warning.
The logic of whether to remove the core or not would be in Node (if
self.config.lcores is either "" or "any").
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index 3f7bc2acae..01a6afdc72 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -163,13 +163,9 @@
> },
> "lcores": {
> "type": "string",
> - "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
> + "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
This should be added to the description.
> "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
> },
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> index 12a40170ac..9b3f01f1e9 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -86,6 +86,15 @@ def __init__(self, node_config: NodeConfiguration):
> self.lcores, LogicalCoreList(self.config.lcores)
> ).filter()
>
> + if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
> + self._logger.info(
If this is a warning, then we should call self._logger.warning()
> + """
> + WARNING: First core being used;
> + using the first core is considered risky and should only
> + be done by advanced users.
> + """
> + )
> +
> self._other_sessions = []
> self._init_ports()
>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 2/6] dts: Use First Core Logic Change
2024-07-05 17:13 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
2024-09-10 13:34 ` Juraj Linkeš
@ 2024-11-18 16:54 ` Luca Vizzarro
1 sibling, 0 replies; 46+ messages in thread
From: Luca Vizzarro @ 2024-11-18 16:54 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev
Same comments as Juraj again. Also the commit subject should be all
lower case, and the bugzilla tag should be split from the signed-off-by
here too.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 3/6] dts: Self-Discovering Architecture Change
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (6 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-09-10 13:41 ` Juraj Linkeš
2024-11-18 17:14 ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
` (3 subsequent siblings)
11 siblings, 2 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
The 'arch' attribute in the conf.yaml is unnecessary, as this can be
readily discovered within the constructor of any given node. Since OS is
determined within user configuration, finding system arch can be done
both reliably and easily within the framework.
For Linux/Posix systems, the 'uname' command is used to determine system
architecture. I believe that this is posix-standard and utilizes a
standardized output.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 2 --
dts/framework/config/__init__.py | 4 ----
dts/framework/config/conf_yaml_schema.json | 12 ------------
dts/framework/config/types.py | 2 --
dts/framework/testbed_model/node.py | 4 +++-
dts/framework/testbed_model/os_session.py | 8 ++++++++
dts/framework/testbed_model/posix_session.py | 6 ++++++
7 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 53192e0761..7ca4c05b55 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -27,7 +27,6 @@ nodes:
- name: "SUT 1"
hostname: sut1.change.me.localhost
user: dtsuser
- arch: x86_64
os: linux
lcores: "" # use all available logical cores (Skips first core)
memory_channels: 4 # tells DPDK to use 4 memory channels
@@ -52,7 +51,6 @@ nodes:
- 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
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 4c05373ef3..662b3070a1 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -208,7 +208,6 @@ class NodeConfiguration:
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.
@@ -220,7 +219,6 @@ class NodeConfiguration:
hostname: str
user: str
password: str | None
- arch: Architecture
os: OS
lcores: str
use_first_core: bool
@@ -257,7 +255,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
@@ -271,7 +268,6 @@ def from_dict(
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
- arch=Architecture(d["arch"]),
os=OS(d["os"]),
lcores=lcores,
use_first_core=use_first_core,
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 01a6afdc72..e65ea45058 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,14 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "ARCH": {
- "type": "string",
- "enum": [
- "x86_64",
- "arm64",
- "ppc64le"
- ]
- },
"OS": {
"type": "string",
"enum": [
@@ -155,9 +147,6 @@
"type": "string",
"description": "The password to use on this node. Use only as a last resort. SSH keys are STRONGLY preferred."
},
- "arch": {
- "$ref": "#/definitions/ARCH"
- },
"os": {
"$ref": "#/definitions/OS"
},
@@ -233,7 +222,6 @@
"name",
"hostname",
"user",
- "arch",
"os"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index 2f75724c5e..9934fef503 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -56,8 +56,6 @@ class NodeConfigDict(TypedDict):
#:
password: str
#:
- arch: str
- #:
os: str
#:
lcores: str
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 9b3f01f1e9..09399f4823 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -17,7 +17,7 @@
from ipaddress import IPv4Interface, IPv6Interface
from typing import Any, Callable, Union
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import OS, Architecture, NodeConfiguration, TestRunConfiguration
from framework.exception import ConfigurationError
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
@@ -55,6 +55,7 @@ class Node(ABC):
main_session: OSSession
config: NodeConfiguration
name: str
+ arch: Architecture
lcores: list[LogicalCore]
ports: list[Port]
_logger: DTSLogger
@@ -77,6 +78,7 @@ def __init__(self, node_config: NodeConfiguration):
self.name = node_config.name
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
+ self.arch = Architecture(self.main_session.get_arch_info())
self._logger.info(f"Connected to node: {self.name}")
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..02277eee1f 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -342,6 +342,14 @@ def get_node_info(self) -> NodeInfo:
Node information.
"""
+ @abstractmethod
+ def get_arch_info(self) -> str:
+ """Discover CPU architecture of the remote host.
+
+ Returns:
+ Remote host CPU architecture.
+ """
+
@abstractmethod
def update_ports(self, ports: list[Port]) -> None:
"""Get additional information about ports from the operating system and update them.
diff --git a/dts/framework/testbed_model/posix_session.py b/dts/framework/testbed_model/posix_session.py
index d279bb8b53..91afca61ea 100644
--- a/dts/framework/testbed_model/posix_session.py
+++ b/dts/framework/testbed_model/posix_session.py
@@ -295,3 +295,9 @@ def get_node_info(self) -> NodeInfo:
).stdout.split("\n")
kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
+
+ def get_arch_info(self) -> str:
+ """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
+ # return str(self.send_command('arch')).stdout
+
+ return str(self.send_command("uname -m").stdout.removesuffix("\n"))
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 3/6] dts: Self-Discovering Architecture Change
2024-07-05 17:13 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
@ 2024-09-10 13:41 ` Juraj Linkeš
2024-11-18 17:14 ` Luca Vizzarro
1 sibling, 0 replies; 46+ messages in thread
From: Juraj Linkeš @ 2024-09-10 13:41 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, luca.vizzarro,
yoan.picchi, Honnappa.Nagarahalli, paul.szczepanek
Cc: dev
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> The 'arch' attribute in the conf.yaml is unnecessary, as this can be
> readily discovered within the constructor of any given node. Since OS is
> determined within user configuration, finding system arch can be done
> both reliably and easily within the framework.
>
> For Linux/Posix systems, the 'uname' command is used to determine system
> architecture. I believe that this is posix-standard and utilizes a
> standardized output.
From what I can tell, uname is it POSIX compliant. Let's reword this to
remove the uncertainty.
> diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
> index 79f56b289b..02277eee1f 100644
> --- a/dts/framework/testbed_model/os_session.py
> +++ b/dts/framework/testbed_model/os_session.py
> @@ -342,6 +342,14 @@ def get_node_info(self) -> NodeInfo:
> Node information.
> """
>
> + @abstractmethod
> + def get_arch_info(self) -> str:
I'd rename this to just get_arch(), as get_arch_info implies we're
getting more than just a string representation of the archirecture.
> + """Discover CPU architecture of the remote host.
The CPU architecture
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 3/6] dts: Self-Discovering Architecture Change
2024-07-05 17:13 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-09-10 13:41 ` Juraj Linkeš
@ 2024-11-18 17:14 ` Luca Vizzarro
1 sibling, 0 replies; 46+ messages in thread
From: Luca Vizzarro @ 2024-11-18 17:14 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev
Here again I am on the same level as Juraj. Repeating my previous
comments on commit subject and Bugzilla ID.
Moreover, the subject should be of imperative form according to the
contributing guidelines. In other words, the first word is always an
imperative verb. Something like this could work:
dts: enable self-discovering architecture
About the commit body indicating uncertainty ("I believe..."), there
shouldn't be space for uncertainty on the tree, but just backed-up
facts. If you are uncertain about something, make sure to find out if
the statement stands true and provide the reasoning around it.
You can find out if the call is a standard by looking up the manual[1].
In this case it doesn't mention anything except of being related to the
syscall[2], which is indeed backed up by the POSIX.1 standard. But you
can attempt to compare it to any other POSIX OS and draw some
conclusions. For example FreeBSD's manual[3], explicitly states that
their command is conform to the POSIX.2 standard. And you'll notice that
FreeBSD's has more options than Linux's. The conclusion I can gather
here is that Linux's version is not entirely conform and implements only
a subset. For our usage, this is good enough as it still falls under the
POSIX umbrella. Therefore, you can change your paragraph into something
like:
The POSIX-compliant options of the command `uname` are used
to determine the system architecture.
Hope this helps!
Best,
Luca
[1] https://man7.org/linux/man-pages/man1/uname.1.html
[2] https://man7.org/linux/man-pages/man2/uname.2.html
[3] https://man.freebsd.org/cgi/man.cgi?uname
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (7 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-09-10 14:04 ` Juraj Linkeš
2024-11-18 17:16 ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
` (2 subsequent siblings)
11 siblings, 2 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
Rework 'lcores' and 'memory_channels' into a new 'dpdk_config'
subsection in an effort to make these attributes SUT specific; the
traffic generator, more often than not, does not need this information.
Ideally, if such information is needed, then it will be listed in the
'traffic_generator' component in TG Node configuration. Such logic is
not introduced in this patch, but the framework can be rewritten to do
so without any implications of extreme effort.
To make this work, use_first_core has been removed from the framework
entirely in favor of doing this within the LogicalCoreListFilter object.
Since use_first_core was only ever activated when logical core 0 was
explicitly defined, core 0 can be removed from the list of total logical
cores assuming that it was not listed within filter_specifier.
This patch also removes 'vdevs' from 'system_under_test_node' and moves
it into 'executions.'
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 20 ++++-----
dts/framework/config/__init__.py | 45 +++++++++++--------
dts/framework/config/conf_yaml_schema.json | 47 ++++++++++----------
dts/framework/config/types.py | 21 ++++++---
dts/framework/testbed_model/cpu.py | 6 ++-
dts/framework/testbed_model/linux_session.py | 5 +--
dts/framework/testbed_model/node.py | 26 +----------
dts/framework/testbed_model/os_session.py | 2 +-
dts/framework/testbed_model/sut_node.py | 12 +++++
9 files changed, 95 insertions(+), 89 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 7ca4c05b55..bb1fbc86e3 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -14,12 +14,11 @@ test_runs:
test_suites: # the following test suites will be run in their entirety
- hello_world
- os_udp
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
# 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 test run
- - "crypto_openssl"
- # Traffic generator node to use for this test run
+ system_under_test_node: "SUT 1"
+ # 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
@@ -28,11 +27,6 @@ nodes:
hostname: sut1.change.me.localhost
user: dtsuser
os: linux
- lcores: "" # use all available logical cores (Skips first core)
- memory_channels: 4 # tells DPDK to use 4 memory channels
- hugepages_2mb: # optional; if removed, will use system hugepage configuration
- number_of: 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"
@@ -46,6 +40,12 @@ nodes:
os_driver: i40e
peer_node: "TG 1"
peer_pci: "0000:00:08.1"
+ hugepages_2mb: # optional; if removed, will use system hugepage configuration
+ number_of: 256
+ force_first_numa: false
+ dpdk_config:
+ lcores: "" # use all available logical cores (Skips first core)
+ memory_channels: 4 # tells DPDK to use 4 memory channels
# 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"
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 662b3070a1..ed1c979fb6 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -209,8 +209,6 @@ class NodeConfiguration:
password: The password of the user. The use of passwords is heavily discouraged.
Please use keys instead.
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.
"""
@@ -220,8 +218,6 @@ class NodeConfiguration:
user: str
password: str | None
os: OS
- lcores: str
- use_first_core: bool
hugepages: HugepageConfiguration | None
ports: list[PortConfig]
@@ -244,9 +240,6 @@ def from_dict(
hugepage_config_dict["force_first_numa"] = False
hugepage_config = HugepageConfiguration(**hugepage_config_dict)
- lcores = "1" if "lcores" not in d else d["lcores"] if "any" not in d["lcores"] else ""
- use_first_core = "0" in lcores
-
# The calls here contain duplicated code which is here because Mypy doesn't
# properly support dictionary unpacking with TypedDicts
if "traffic_generator" in d:
@@ -256,36 +249,54 @@ def from_dict(
user=d["user"],
password=d.get("password"),
os=OS(d["os"]),
- lcores=lcores,
- use_first_core=use_first_core,
hugepages=hugepage_config,
ports=[PortConfig.from_dict(d["name"], port) for port in d["ports"]],
traffic_generator=TrafficGeneratorConfig.from_dict(d["traffic_generator"]),
)
else:
+ dpdk_config = d["dpdk_config"]
+ dpdk_config["lcores"] = (
+ "1"
+ if "lcores" not in dpdk_config
+ else dpdk_config["lcores"]
+ if "any" not in dpdk_config["lcores"]
+ else ""
+ )
+ dpdk_config["memory_channels"] = dpdk_config.get("memory_channels", 1)
return SutNodeConfiguration(
name=d["name"],
hostname=d["hostname"],
user=d["user"],
password=d.get("password"),
os=OS(d["os"]),
- lcores=lcores,
- use_first_core=use_first_core,
+ dpdk_config=DPDKConfig(**dpdk_config),
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 DPDKConfig:
+ """EAL parameters for executing and running DPDK.
+
+ Attributes:
+ lcores: Logical cores to be used for DPDK execution.
+ memory_channels: Memory channels to be used for DPDK execution.
+ """
+
+ lcores: str
+ memory_channels: int
+
+
@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.
+ dpdk_config: DPDK configuration attributes to be used during execution.
"""
- memory_channels: int
+ dpdk_config: DPDKConfig
@dataclass(slots=True, frozen=True)
@@ -450,7 +461,7 @@ def from_dict(
map(BuildTargetConfiguration.from_dict, d["build_targets"])
)
test_suites: list[TestSuiteConfig] = list(map(TestSuiteConfig.from_dict, d["test_suites"]))
- sut_name = d["system_under_test_node"]["node_name"]
+ sut_name = d["system_under_test_node"]
skip_smoke_tests = d.get("skip_smoke_tests", False)
assert sut_name in node_map, f"Unknown SUT {sut_name} in test run {d}"
system_under_test_node = node_map[sut_name]
@@ -465,9 +476,7 @@ def from_dict(
traffic_generator_node, TGNodeConfiguration
), f"Invalid TG configuration {traffic_generator_node}"
- vdevs = (
- d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
- )
+ vdevs = d["vdevs"] if "vdevs" in d else []
return cls(
build_targets=build_targets,
perf=d["perf"],
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index e65ea45058..b31f4d8dbe 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -150,14 +150,21 @@
"os": {
"$ref": "#/definitions/OS"
},
- "lcores": {
- "type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
- "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
- },
- "memory_channels": {
- "type": "integer",
- "description": "How many memory channels to use. Optional, defaults to 1."
+ "dpdk_config": {
+ "type": "object",
+ "description": "EAL arguments for DPDK execution",
+ "properties": {
+ "lcores": {
+ "type": "string",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
+ "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
+ },
+ "memory_channels": {
+ "type": "integer",
+ "description": "How many memory channels to use. Optional, defaults to 1."
+ }
+ },
+ "minimum": 1
},
"hugepages_2mb": {
"$ref": "#/definitions/hugepages_2mb"
@@ -264,23 +271,15 @@
"description": "Optional field that allows you to skip smoke testing",
"type": "boolean"
},
+ "vdevs": {
+ "description": "Optional list of names of vdevs to be used in execution",
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
"system_under_test_node": {
- "type":"object",
- "properties": {
- "node_name": {
- "$ref": "#/definitions/node_name"
- },
- "vdevs": {
- "description": "Optional list of names of vdevs to be used in the test run",
- "type": "array",
- "items": {
- "type": "string"
- }
- }
- },
- "required": [
- "node_name"
- ]
+ "$ref": "#/definitions/node_name"
},
"traffic_generator_node": {
"$ref": "#/definitions/node_name"
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index 9934fef503..004fc2b2d3 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -33,6 +33,15 @@ class TrafficGeneratorConfigDict(TypedDict):
type: str
+class DPDKConfigDict(TypedDict):
+ """Allowed keys and values."""
+
+ #:
+ memory_channels: int
+ #:
+ lcores: str
+
+
class HugepageConfigurationDict(TypedDict):
"""Allowed keys and values."""
@@ -58,15 +67,11 @@ class NodeConfigDict(TypedDict):
#:
os: str
#:
- lcores: str
- #:
- use_first_core: bool
- #:
ports: list[PortConfigDict]
#:
- memory_channels: int
- #:
traffic_generator: TrafficGeneratorConfigDict
+ #:
+ dpdk_config: DPDKConfigDict
class BuildTargetConfigDict(TypedDict):
@@ -110,9 +115,11 @@ class TestRunConfigDict(TypedDict):
#:
test_suites: TestSuiteConfigDict
#:
- system_under_test_node: TestRunSUTConfigDict
+ system_under_test_node: str
#:
traffic_generator_node: str
+ #:
+ vdevs: list[str]
class ConfigurationDict(TypedDict):
diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py
index a50cf44c19..cc4ca40ad9 100644
--- a/dts/framework/testbed_model/cpu.py
+++ b/dts/framework/testbed_model/cpu.py
@@ -167,7 +167,6 @@ def __init__(
# sorting by core is needed in case hyperthreading is enabled
self._lcores_to_filter = sorted(lcore_list, key=lambda x: x.core, reverse=not ascending)
- self.filter()
@abstractmethod
def filter(self) -> list[LogicalCore]:
@@ -210,6 +209,8 @@ def filter(self) -> list[LogicalCore]:
Returns:
The filtered cores.
"""
+ if 0 in self._lcores_to_filter:
+ self._lcores_to_filter = self._lcores_to_filter[1:]
sockets_to_filter = self._filter_sockets(self._lcores_to_filter)
filtered_lcores = []
for socket_to_filter in sockets_to_filter:
@@ -328,6 +329,9 @@ def filter(self) -> list[LogicalCore]:
Return:
The filtered logical CPU cores.
"""
+ if 0 not in self._filter_specifier.lcore_list:
+ self._lcores_to_filter = self._lcores_to_filter[1:]
+
if not len(self._filter_specifier.lcore_list):
return self._lcores_to_filter
diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 99abc21353..347d01878c 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -68,15 +68,12 @@ class LinuxSession(PosixSession):
def _get_privileged_command(command: str) -> str:
return f"sudo -- sh -c '{command}'"
- def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
+ def get_remote_cpus(self) -> list[LogicalCore]:
"""Overrides :meth:`~.os_session.OSSession.get_remote_cpus`."""
cpu_info = self.send_command("lscpu -p=CPU,CORE,SOCKET,NODE|grep -v \\#").stdout
lcores = []
for cpu_line in cpu_info.splitlines():
lcore, core, socket, node = map(int, cpu_line.split(","))
- if core == 0 and socket == 0 and not use_first_core:
- self._logger.info("Not using the first physical core.")
- continue
lcores.append(LogicalCore(lcore, core, socket, node))
return lcores
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 09399f4823..9630407247 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -22,13 +22,7 @@
from framework.logger import DTSLogger, get_dts_logger
from framework.settings import SETTINGS
-from .cpu import (
- LogicalCore,
- LogicalCoreCount,
- LogicalCoreList,
- LogicalCoreListFilter,
- lcore_filter,
-)
+from .cpu import LogicalCore, LogicalCoreCount, LogicalCoreList, lcore_filter
from .linux_session import LinuxSession
from .os_session import OSSession
from .port import Port
@@ -79,24 +73,8 @@ def __init__(self, node_config: NodeConfiguration):
self._logger = get_dts_logger(self.name)
self.main_session = create_session(self.config, self.name, self._logger)
self.arch = Architecture(self.main_session.get_arch_info())
-
self._logger.info(f"Connected to node: {self.name}")
-
self._get_remote_cpus()
- # filter the node lcores according to the test run configuration
- self.lcores = LogicalCoreListFilter(
- self.lcores, LogicalCoreList(self.config.lcores)
- ).filter()
-
- if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
- self._logger.info(
- """
- WARNING: First core being used;
- using the first core is considered risky and should only
- be done by advanced users.
- """
- )
-
self._other_sessions = []
self._init_ports()
@@ -182,7 +160,7 @@ def filter_lcores(
def _get_remote_cpus(self) -> None:
"""Scan CPUs in the remote OS and store a list of LogicalCores."""
self._logger.info("Getting CPU information.")
- self.lcores = self.main_session.get_remote_cpus(self.config.use_first_core)
+ self.lcores = self.main_session.get_remote_cpus()
def _setup_hugepages(self) -> None:
"""Setup hugepages on the node.
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 02277eee1f..f217a40e7f 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -280,7 +280,7 @@ def get_dpdk_version(self, version_path: str | PurePath) -> str:
"""
@abstractmethod
- def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
+ def get_remote_cpus(self) -> list[LogicalCore]:
r"""Get the list of :class:`~.cpu.LogicalCore`\s on the remote node.
Args:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index a4511157b7..178535b617 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -29,6 +29,7 @@
from framework.settings import SETTINGS
from framework.utils import MesonArgs
+from .cpu import LogicalCore, LogicalCoreList
from .node import Node
from .os_session import OSSession
from .virtual_device import VirtualDevice
@@ -75,6 +76,17 @@ def __init__(self, node_config: SutNodeConfiguration):
node_config: The SUT node's test run configuration.
"""
super().__init__(node_config)
+ self.lcores = self.filter_lcores(LogicalCoreList(self.config.dpdk_config.lcores))
+ if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
+ self._logger.info(
+ """
+ WARNING: First core being used;
+ using the first core is considered risky and should only
+ be done by advanced users.
+ """
+ )
+ else:
+ self._logger.info("Not using first core")
self.virtual_devices = []
self.dpdk_prefix_list = []
self._build_target_config = None
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config
2024-07-05 17:13 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
@ 2024-09-10 14:04 ` Juraj Linkeš
2024-11-18 17:16 ` Luca Vizzarro
1 sibling, 0 replies; 46+ messages in thread
From: Juraj Linkeš @ 2024-09-10 14:04 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, luca.vizzarro,
yoan.picchi, Honnappa.Nagarahalli, paul.szczepanek
Cc: dev
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index e65ea45058..b31f4d8dbe 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -150,14 +150,21 @@
> "os": {
> "$ref": "#/definitions/OS"
> },
> - "lcores": {
> - "type": "string",
> - "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
> - "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
> - },
> - "memory_channels": {
> - "type": "integer",
> - "description": "How many memory channels to use. Optional, defaults to 1."
> + "dpdk_config": {
> + "type": "object",
> + "description": "EAL arguments for DPDK execution",
> + "properties": {
> + "lcores": {
> + "type": "string",
> + "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$|any",
> + "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
> + },
> + "memory_channels": {
> + "type": "integer",
> + "description": "How many memory channels to use. Optional, defaults to 1."
> + }
> + },
> + "minimum": 1
> },
> "hugepages_2mb": {
> "$ref": "#/definitions/hugepages_2mb"
> @@ -264,23 +271,15 @@
> "description": "Optional field that allows you to skip smoke testing",
> "type": "boolean"
> },
> + "vdevs": {
> + "description": "Optional list of names of vdevs to be used in execution",
Don't forget to check these patches for execution -> test run. Also,
while we're at it, let's add a dot to the end of this sentence.
> diff --git a/dts/framework/testbed_model/cpu.py b/dts/framework/testbed_model/cpu.py
> index a50cf44c19..cc4ca40ad9 100644
> --- a/dts/framework/testbed_model/cpu.py
> +++ b/dts/framework/testbed_model/cpu.py
> @@ -167,7 +167,6 @@ def __init__(
>
> # sorting by core is needed in case hyperthreading is enabled
> self._lcores_to_filter = sorted(lcore_list, key=lambda x: x.core, reverse=not ascending)
> - self.filter()
This is a pretty good catch.
>
> @abstractmethod
> def filter(self) -> list[LogicalCore]:
> @@ -210,6 +209,8 @@ def filter(self) -> list[LogicalCore]:
> Returns:
> The filtered cores.
> """
> + if 0 in self._lcores_to_filter:
> + self._lcores_to_filter = self._lcores_to_filter[1:]
> sockets_to_filter = self._filter_sockets(self._lcores_to_filter)
> filtered_lcores = []
> for socket_to_filter in sockets_to_filter:
> @@ -328,6 +329,9 @@ def filter(self) -> list[LogicalCore]:
> Return:
> The filtered logical CPU cores.
> """
> + if 0 not in self._filter_specifier.lcore_list:
> + self._lcores_to_filter = self._lcores_to_filter[1:]
> +
I see you actually did what I was thinking of. This should be in 2/6.
I'm not a big fan of two different implementation of what should be
common logic. Can we pass skip_first_core to
LogicalCoreFilter.__init__() and remove the first core from
self._lcores_to_filter if True (it would default to False)? skip_first
core would be determined in SutNode.__init__() and then passed downstream.
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index a4511157b7..178535b617 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -29,6 +29,7 @@
> from framework.settings import SETTINGS
> from framework.utils import MesonArgs
>
> +from .cpu import LogicalCore, LogicalCoreList
> from .node import Node
> from .os_session import OSSession
> from .virtual_device import VirtualDevice
> @@ -75,6 +76,17 @@ def __init__(self, node_config: SutNodeConfiguration):
> node_config: The SUT node's test run configuration.
> """
> super().__init__(node_config)
Here is where we could determine use_first_core from
self.config.dpdk_config.lcores and pass that to self.filter_lcores().
> + self.lcores = self.filter_lcores(LogicalCoreList(self.config.dpdk_config.lcores))
> + if LogicalCore(lcore=0, core=0, socket=0, node=0) in self.lcores:
> + self._logger.info(
> + """
> + WARNING: First core being used;
> + using the first core is considered risky and should only
> + be done by advanced users.
> + """
> + )
> + else:
> + self._logger.info("Not using first core")
> self.virtual_devices = []
> self.dpdk_prefix_list = []
> self._build_target_config = None
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config
2024-07-05 17:13 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-09-10 14:04 ` Juraj Linkeš
@ 2024-11-18 17:16 ` Luca Vizzarro
1 sibling, 0 replies; 46+ messages in thread
From: Luca Vizzarro @ 2024-11-18 17:16 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev
Good one. My only comment – aside the usual ones – on top of Juraj's is
could try to make the commit subject more concise. For example:
dts: isolate config of dpdk arguments
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 5/6] dts: add conditional behavior for test suite
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (8 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-07-16 14:59 ` Jeremy Spewock
` (3 more replies)
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2024-07-05 18:24 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
11 siblings, 4 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
There is some odd functionality/behavior in how the --test-suite
parameters interacts in conjunction with the 'test_suites' attribute in
the config file. If a user leaves an empty list underneath
'test_suites,' or if they negate the attribute entirely, even if said
user adds test suites via the --test-suite parameter, a schema violation
is thrown.
This patch mitigates this, by removing the schema requirement if the
user has indicated test suites within main.py parameters, allowing for
the 'test_suites' attribute to be optional.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/framework/config/__init__.py | 7 ++++++-
dts/framework/runner.py | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index ed1c979fb6..82182b5c99 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -553,7 +553,7 @@ def from_dict(cls, d: ConfigurationDict) -> Self:
return cls(test_runs=test_runs)
-def load_config(config_file_path: Path) -> Configuration:
+def load_config(config_file_path: Path, test_suites: list[TestSuiteConfig]) -> Configuration:
"""Load DTS test run configuration from a file.
Load the YAML test run configuration file
@@ -576,6 +576,11 @@ def load_config(config_file_path: Path) -> Configuration:
with open(schema_path, "r") as f:
schema = json.load(f)
+ if test_suites:
+ schema["properties"]["test_runs"]["items"]["required"].remove("test_suites")
+ for test_run in config_data["test_runs"]:
+ if not hasattr(test_run, "test_suites"):
+ test_run["test_suites"] = []
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/runner.py b/dts/framework/runner.py
index 2a1019899a..edda5510af 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -85,7 +85,7 @@ class DTSRunner:
def __init__(self):
"""Initialize the instance with configuration, logger, result and string constants."""
- self._configuration = load_config(SETTINGS.config_file_path)
+ self._configuration = load_config(SETTINGS.config_file_path, SETTINGS.test_suites)
self._logger = get_dts_logger()
if not os.path.exists(SETTINGS.output_dir):
os.makedirs(SETTINGS.output_dir)
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 5/6] dts: add conditional behavior for test suite
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
@ 2024-07-16 14:59 ` Jeremy Spewock
2024-09-10 14:12 ` Juraj Linkeš
` (2 subsequent siblings)
3 siblings, 0 replies; 46+ messages in thread
From: Jeremy Spewock @ 2024-07-16 14:59 UTC (permalink / raw)
To: Nicholas Pratte
Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
paul.szczepanek, juraj.linkes, dev
I think it makes sense to allow users to not specify any test suites
in the config file if they specify them through other means, so I like
this change.
On Fri, Jul 5, 2024 at 1:20 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> There is some odd functionality/behavior in how the --test-suite
> parameters interacts in conjunction with the 'test_suites' attribute in
> the config file. If a user leaves an empty list underneath
> 'test_suites,' or if they negate the attribute entirely, even if said
> user adds test suites via the --test-suite parameter, a schema violation
> is thrown.
>
> This patch mitigates this, by removing the schema requirement if the
> user has indicated test suites within main.py parameters, allowing for
> the 'test_suites' attribute to be optional.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 5/6] dts: add conditional behavior for test suite
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-16 14:59 ` Jeremy Spewock
@ 2024-09-10 14:12 ` Juraj Linkeš
2024-11-06 20:52 ` Dean Marx
2024-11-18 17:21 ` Luca Vizzarro
3 siblings, 0 replies; 46+ messages in thread
From: Juraj Linkeš @ 2024-09-10 14:12 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, luca.vizzarro,
yoan.picchi, Honnappa.Nagarahalli, paul.szczepanek
Cc: dev
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> There is some odd functionality/behavior in how the --test-suite
> parameters interacts in conjunction with the 'test_suites' attribute in
> the config file. If a user leaves an empty list underneath
> 'test_suites,' or if they negate the attribute entirely, even if said
> user adds test suites via the --test-suite parameter, a schema violation
> is thrown.
>
> This patch mitigates this, by removing the schema requirement if the
> user has indicated test suites within main.py parameters, allowing for
> the 'test_suites' attribute to be optional.
>
Nice idea, doesn't look like there's any hard in adding it. Should we
document this (the fact that it could be optional under certain
circumstances) somewhere, like add something to the description in schema?
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
> dts/framework/config/__init__.py | 7 ++++++-
> dts/framework/runner.py | 2 +-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index ed1c979fb6..82182b5c99 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -553,7 +553,7 @@ def from_dict(cls, d: ConfigurationDict) -> Self:
> return cls(test_runs=test_runs)
>
>
> -def load_config(config_file_path: Path) -> Configuration:
> +def load_config(config_file_path: Path, test_suites: list[TestSuiteConfig]) -> Configuration:
The variable should maybe contain that these test suites are not from
config_file_path. Maybe just rename to other_test_suites? Or something
like that.
Also, it needs to be added to Args:
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 5/6] dts: add conditional behavior for test suite
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-16 14:59 ` Jeremy Spewock
2024-09-10 14:12 ` Juraj Linkeš
@ 2024-11-06 20:52 ` Dean Marx
2024-11-18 17:21 ` Luca Vizzarro
3 siblings, 0 replies; 46+ messages in thread
From: Dean Marx @ 2024-11-06 20:52 UTC (permalink / raw)
To: Nicholas Pratte
Cc: probb, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes, dev
[-- Attachment #1: Type: text/plain, Size: 774 bytes --]
On Fri, Jul 5, 2024 at 1:20 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> There is some odd functionality/behavior in how the --test-suite
> parameters interacts in conjunction with the 'test_suites' attribute in
> the config file. If a user leaves an empty list underneath
> 'test_suites,' or if they negate the attribute entirely, even if said
> user adds test suites via the --test-suite parameter, a schema violation
> is thrown.
>
> This patch mitigates this, by removing the schema requirement if the
> user has indicated test suites within main.py parameters, allowing for
> the 'test_suites' attribute to be optional.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 1209 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 5/6] dts: add conditional behavior for test suite
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
` (2 preceding siblings ...)
2024-11-06 20:52 ` Dean Marx
@ 2024-11-18 17:21 ` Luca Vizzarro
3 siblings, 0 replies; 46+ messages in thread
From: Luca Vizzarro @ 2024-11-18 17:21 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev
The issue you mention should be resolved now with the Pydantic changes.
Do we still have reason to make this change? I guess we could make the
test suites in the config file optional, in which case you can just add
a default to the field and remove the constraint.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 6/6] doc: dpdk documentation changes for new dts config
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (9 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
@ 2024-07-05 17:13 ` Nicholas Pratte
2024-09-10 14:17 ` Juraj Linkeš
` (2 more replies)
2024-07-05 18:24 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
11 siblings, 3 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 17:13 UTC (permalink / raw)
To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev, Nicholas Pratte
Adjusted DPDK documentation to reflect the changes made to the dts
conf.yaml configuration file.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
doc/guides/tools/dts.rst | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 515b15e4d8..a64580e0de 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -437,14 +437,6 @@ _`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``.
@@ -456,9 +448,6 @@ _`Build target`
*mapping* – 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.
@@ -565,18 +554,15 @@ involved in the testing. These can be defined with the following mappings:
| | |
| | **NB**: Use only as last resort. SSH keys are **strongly** preferred. |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``arch`` | The architecture of this node. See `ARCH`_ for supported values. |
- +-----------------------+---------------------------------------------------------------------------------------+
| ``os`` | The operating system of this node. See `OS`_ for supported values. |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``lcores`` | | (*optional*, defaults to 1) *string* – Comma-separated list of logical |
- | | | cores to use. An empty string means use all lcores. |
- | | |
- | | **Example**: ``1,2,3,4,5,18-22`` |
+ | ``dpdk_config`` | Configuration relating to DPDK (to be specified on SUT Nodes) |
+-----------------------+---------------------------------------------------------------------------------------+
- | ``use_first_core`` | (*optional*, defaults to ``false``) *boolean* |
+ | ``lcores`` | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical |
+ | | | cores to use. An empty string means use all lcores except core 0. core 0 is used |
+ | | | only when explicitly specified |
| | |
- | | Indicates whether DPDK should use only the first physical core or not. |
+ | | **Example**: ``1,2,3,4,5,18-22`` |
+-----------------------+---------------------------------------------------------------------------------------+
| ``memory_channels`` | (*optional*, defaults to 1) *integer* |
| | |
@@ -617,4 +603,4 @@ And they both have two network ports which are physically connected to each othe
.. literalinclude:: ../../../dts/conf.yaml
:language: yaml
- :start-at: test_runs:
+ :start-at: test_runs:
\ No newline at end of file
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v2 6/6] doc: dpdk documentation changes for new dts config
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
@ 2024-09-10 14:17 ` Juraj Linkeš
2024-11-06 20:57 ` Dean Marx
2024-11-18 17:21 ` Luca Vizzarro
2 siblings, 0 replies; 46+ messages in thread
From: Juraj Linkeš @ 2024-09-10 14:17 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, luca.vizzarro,
yoan.picchi, Honnappa.Nagarahalli, paul.szczepanek
Cc: dev
On 5. 7. 2024 19:13, Nicholas Pratte wrote:
> Adjusted DPDK documentation to reflect the changes made to the dts
> conf.yaml configuration file.
>
Looking at the changes to conf.yaml, looks like there are missing pieces
(such as moved vdevs and memory_channels), but the patch no longer
applies and it's hard to verify without that. But this is likely not
needed because the Pydantic changes are going to simplify this.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 6/6] doc: dpdk documentation changes for new dts config
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2024-09-10 14:17 ` Juraj Linkeš
@ 2024-11-06 20:57 ` Dean Marx
2024-11-18 17:21 ` Luca Vizzarro
2 siblings, 0 replies; 46+ messages in thread
From: Dean Marx @ 2024-11-06 20:57 UTC (permalink / raw)
To: Nicholas Pratte
Cc: probb, jspewock, luca.vizzarro, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes, dev
[-- Attachment #1: Type: text/plain, Size: 313 bytes --]
On Fri, Jul 5, 2024 at 1:20 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> Adjusted DPDK documentation to reflect the changes made to the dts
> conf.yaml configuration file.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 707 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 6/6] doc: dpdk documentation changes for new dts config
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2024-09-10 14:17 ` Juraj Linkeš
2024-11-06 20:57 ` Dean Marx
@ 2024-11-18 17:21 ` Luca Vizzarro
2 siblings, 0 replies; 46+ messages in thread
From: Luca Vizzarro @ 2024-11-18 17:21 UTC (permalink / raw)
To: Nicholas Pratte, probb, dmarx, jspewock, yoan.picchi,
Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
Cc: dev
As Juraj already said this is no longer needed.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 1/6] dts: Remove build target config and list of devices
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
` (10 preceding siblings ...)
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
@ 2024-07-05 18:24 ` Nicholas Pratte
11 siblings, 0 replies; 46+ messages in thread
From: Nicholas Pratte @ 2024-07-05 18:24 UTC (permalink / raw)
To: dmarx, paul.szczepanek, juraj.linkes, yoan.picchi, jspewock,
luca.vizzarro, probb, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
Remove the list of devices from the schema, as these are unuesed.
Likewise, removed build-target information since these is not currently
used, and it is unlikely to be used in the future. Adjustments to the
dts.rst are made to reflect these changes.
Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/conf.yaml | 5 +-
dts/framework/config/__init__.py | 30 +-------
dts/framework/config/conf_yaml_schema.json | 79 ----------------------
dts/framework/config/types.py | 6 --
dts/framework/runner.py | 2 +-
dts/framework/test_result.py | 14 +---
dts/framework/testbed_model/sut_node.py | 8 +--
7 files changed, 5 insertions(+), 139 deletions(-)
diff --git a/dts/conf.yaml b/dts/conf.yaml
index 7d95016e68..56cc08ced2 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -5,11 +5,8 @@
test_runs:
# define one test run environment
- build_targets:
- - arch: x86_64
- os: linux
- cpu: native
# the combination of the following two makes CC="ccache gcc"
- compiler: gcc
+ - compiler: gcc
compiler_wrapper: ccache
perf: false # disable performance testing
func: true # enable functional testing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..456a8a83ab 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -85,22 +85,6 @@ class OS(StrEnum):
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."""
@@ -341,28 +325,20 @@ class BuildTargetConfiguration:
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
compiler: Compiler
compiler_wrapper: str
- name: str
@classmethod
def from_dict(cls, d: BuildTargetConfigDict) -> Self:
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`.
+ `compiler` is converted to :class:`Enum`\s
Args:
d: The configuration dictionary.
@@ -371,12 +347,8 @@ def from_dict(cls, d: BuildTargetConfigDict) -> Self:
The build target configuration instance.
"""
return cls(
- arch=Architecture(d["arch"]),
- os=OS(d["os"]),
- cpu=CPUType(d["cpu"]),
compiler=Compiler(d["compiler"]),
compiler_wrapper=d.get("compiler_wrapper", ""),
- name=f"{d['arch']}-{d['os']}-{d['cpu']}-{d['compiler']}",
)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..3f7bc2acae 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,76 +6,6 @@
"type": "string",
"description": "A unique identifier for a node"
},
- "NIC": {
- "type": "string",
- "enum": [
- "ALL",
- "ConnectX3_MT4103",
- "ConnectX4_LX_MT4117",
- "ConnectX4_MT4115",
- "ConnectX5_MT4119",
- "ConnectX5_MT4121",
- "I40E_10G-10G_BASE_T_BC",
- "I40E_10G-10G_BASE_T_X722",
- "I40E_10G-SFP_X722",
- "I40E_10G-SFP_XL710",
- "I40E_10G-X722_A0",
- "I40E_1G-1G_BASE_T_X722",
- "I40E_25G-25G_SFP28",
- "I40E_40G-QSFP_A",
- "I40E_40G-QSFP_B",
- "IAVF-ADAPTIVE_VF",
- "IAVF-VF",
- "IAVF_10G-X722_VF",
- "ICE_100G-E810C_QSFP",
- "ICE_25G-E810C_SFP",
- "ICE_25G-E810_XXV_SFP",
- "IGB-I350_VF",
- "IGB_1G-82540EM",
- "IGB_1G-82545EM_COPPER",
- "IGB_1G-82571EB_COPPER",
- "IGB_1G-82574L",
- "IGB_1G-82576",
- "IGB_1G-82576_QUAD_COPPER",
- "IGB_1G-82576_QUAD_COPPER_ET2",
- "IGB_1G-82580_COPPER",
- "IGB_1G-I210_COPPER",
- "IGB_1G-I350_COPPER",
- "IGB_1G-I354_SGMII",
- "IGB_1G-PCH_LPTLP_I218_LM",
- "IGB_1G-PCH_LPTLP_I218_V",
- "IGB_1G-PCH_LPT_I217_LM",
- "IGB_1G-PCH_LPT_I217_V",
- "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
- "IGC-I225_LM",
- "IGC-I226_LM",
- "IXGBE_10G-82599_SFP",
- "IXGBE_10G-82599_SFP_SF_QP",
- "IXGBE_10G-82599_T3_LOM",
- "IXGBE_10G-82599_VF",
- "IXGBE_10G-X540T",
- "IXGBE_10G-X540_VF",
- "IXGBE_10G-X550EM_A_SFP",
- "IXGBE_10G-X550EM_X_10G_T",
- "IXGBE_10G-X550EM_X_SFP",
- "IXGBE_10G-X550EM_X_VF",
- "IXGBE_10G-X550T",
- "IXGBE_10G-X550_VF",
- "brcm_57414",
- "brcm_P2100G",
- "cavium_0011",
- "cavium_a034",
- "cavium_a063",
- "cavium_a064",
- "fastlinq_ql41000",
- "fastlinq_ql41000_vf",
- "fastlinq_ql45000",
- "fastlinq_ql45000_vf",
- "hi1822",
- "virtio"
- ]
- },
-
"ARCH": {
"type": "string",
"enum": [
@@ -124,12 +54,6 @@
"other"
]
},
- "os": {
- "$ref": "#/definitions/OS"
- },
- "cpu": {
- "$ref": "#/definitions/cpu"
- },
"compiler": {
"$ref": "#/definitions/compiler"
},
@@ -140,9 +64,6 @@
},
"additionalProperties": false,
"required": [
- "arch",
- "os",
- "cpu",
"compiler"
]
},
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index cf16556403..2f75724c5e 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -74,12 +74,6 @@ class NodeConfigDict(TypedDict):
class BuildTargetConfigDict(TypedDict):
"""Allowed keys and values."""
- #:
- arch: str
- #:
- os: str
- #:
- cpu: str
#:
compiler: str
#:
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..2a1019899a 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -480,7 +480,7 @@ def _run_build_target(
test_suites_with_cases: The test suites with test cases to run.
"""
self._logger.set_stage(DtsStage.build_target_setup)
- self._logger.info(f"Running build target '{build_target_config.name}'.")
+ self._logger.info("Running build target.")
try:
sut_node.set_up_build_target(build_target_config)
diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
index 5694a2482b..7fcc24fecd 100644
--- a/dts/framework/test_result.py
+++ b/dts/framework/test_result.py
@@ -31,12 +31,9 @@
from typing import Union
from .config import (
- OS,
- Architecture,
BuildTargetConfiguration,
BuildTargetInfo,
Compiler,
- CPUType,
NodeInfo,
TestRunConfiguration,
TestSuiteConfig,
@@ -223,7 +220,7 @@ class DTSResult(BaseResult):
"""Stores environment information and test results from a DTS run.
* Test run level information, such as testbed and the test suite list,
- * Build target level information, such as compiler, target OS and cpu,
+ * Build target level compiler information
* Test suite and test case results,
* All errors that are caught and recorded during DTS execution.
@@ -405,17 +402,11 @@ class BuildTargetResult(BaseResult):
The internal list stores the results of all test suites in a given build target.
Attributes:
- arch: The DPDK build target architecture.
- os: The DPDK build target operating system.
- cpu: The DPDK build target CPU.
compiler: The DPDK build target compiler.
compiler_version: The DPDK build target compiler version.
dpdk_version: The built DPDK version.
"""
- arch: Architecture
- os: OS
- cpu: CPUType
compiler: Compiler
compiler_version: str | None
dpdk_version: str | None
@@ -433,9 +424,6 @@ def __init__(
build_target_config: The build target's test run configuration.
"""
super().__init__()
- self.arch = build_target_config.arch
- self.os = build_target_config.os
- self.cpu = build_target_config.cpu
self.compiler = build_target_config.compiler
self.compiler_version = None
self.dpdk_version = None
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 2855fe0276..a4511157b7 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -115,12 +115,7 @@ def remote_dpdk_build_dir(self) -> PurePath:
This is the directory where DPDK was built.
We assume it was built in a subdirectory of the extracted tarball.
"""
- if self._build_target_config:
- return self.main_session.join_remote_path(
- self._remote_dpdk_dir, self._build_target_config.name
- )
- else:
- return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
+ return self.main_session.join_remote_path(self._remote_dpdk_dir, "build")
@property
def dpdk_version(self) -> str:
@@ -217,7 +212,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
"""Populate common environment variables and set build target config."""
self._env_vars = {}
self._build_target_config = build_target_config
- self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
self._env_vars["CC"] = build_target_config.compiler.name
if build_target_config.compiler_wrapper:
self._env_vars["CC"] = (
--
2.44.0
^ permalink raw reply [flat|nested] 46+ messages in thread