Reviewed-by: Nicholas Pratte <npra...@iol.unh.edu>
On Fri, Jan 24, 2025 at 6:39 AM Luca Vizzarro <luca.vizza...@arm.com> wrote:
>
> From: Nicholas Pratte <npra...@iol.unh.edu>
>
> 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 'test_runs'.
>
> Bugzilla ID: 1360
>
> Signed-off-by: Nicholas Pratte <npra...@iol.unh.edu>
> Signed-off-by: Luca Vizzarro <luca.vizza...@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepa...@arm.com>
> Reviewed-by: Dean Marx <dm...@iol.unh.edu>
> ---
> dts/conf.yaml | 20 +++++-----
> dts/framework/config/__init__.py | 39 ++++++++++----------
> dts/framework/runner.py | 6 +--
> dts/framework/testbed_model/cpu.py | 6 ++-
> dts/framework/testbed_model/linux_session.py | 5 +--
> dts/framework/testbed_model/node.py | 27 +-------------
> dts/framework/testbed_model/os_session.py | 2 +-
> dts/framework/testbed_model/sut_node.py | 14 ++++++-
> 8 files changed, 54 insertions(+), 65 deletions(-)
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index c93eedbc94..bc78882d0d 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -26,12 +26,11 @@ test_runs:
> skip_smoke_tests: false # optional
> test_suites: # the following test suites will be run in their entirety
> - hello_world
> + 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
> @@ -40,11 +39,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"@0000:00:08.0 and "TG
> 1"@0000:00:08.0
> - pci: "0000:00:08.0"
> @@ -58,6 +52,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 5dfa0cf0d4..2496f48e20 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -161,15 +161,23 @@ class NodeConfiguration(FrozenModel):
> password: str | None = None
> #: The operating system of the
> :class:`~framework.testbed_model.node.Node`.
> os: OS
> - #: A comma delimited list of logical cores to use when running DPDK.
> ```any```, an empty
> - #: string or omitting this field means use any core except for the first
> one. The first core
> - #: will only be used if explicitly set.
> - lcores: LogicalCores = ""
> #: An optional hugepage configuration.
> hugepages: HugepageConfiguration | None = Field(None,
> alias="hugepages_2mb")
> #: The ports that can be used in testing.
> ports: list[PortConfig] = Field(min_length=1)
>
> +
> +class DPDKConfiguration(FrozenModel):
> + """Configuration of the DPDK EAL parameters."""
> +
> + #: A comma delimited list of logical cores to use when running DPDK.
> ```any```, an empty
> + #: string or omitting this field means use any core except for the first
> one. The first core
> + #: will only be used if explicitly set.
> + lcores: LogicalCores = ""
> +
> + #: The number of memory channels to use when running DPDK.
> + memory_channels: int = 1
> +
> @property
> def use_first_core(self) -> bool:
> """Returns :data:`True` if `lcores` explicitly selects the first
> core."""
> @@ -179,8 +187,8 @@ def use_first_core(self) -> bool:
> class SutNodeConfiguration(NodeConfiguration):
> """:class:`~framework.testbed_model.sut_node.SutNode` specific
> configuration."""
>
> - #: The number of memory channels to use when running DPDK.
> - memory_channels: int = 1
> + #: The runtime configuration for DPDK.
> + dpdk_config: DPDKConfiguration
>
>
> class TGNodeConfiguration(NodeConfiguration):
> @@ -405,15 +413,6 @@ def validate_names(self) -> Self:
> return self
>
>
> -class TestRunSUTNodeConfiguration(FrozenModel):
> - """The SUT node configuration of a test run."""
> -
> - #: The SUT node to use in this test run.
> - node_name: str
> - #: The names of virtual devices to test.
> - vdevs: list[str] = Field(default_factory=list)
> -
> -
> class TestRunConfiguration(FrozenModel):
> """The configuration of a test run.
>
> @@ -431,10 +430,12 @@ class TestRunConfiguration(FrozenModel):
> skip_smoke_tests: bool = False
> #: The names of test suites and/or test cases to execute.
> test_suites: list[TestSuiteConfig] = Field(min_length=1)
> - #: The SUT node configuration to use in this test run.
> - system_under_test_node: TestRunSUTNodeConfiguration
> + #: The SUT node name to use in this test run.
> + system_under_test_node: str
> #: The TG node name to use in this test run.
> traffic_generator_node: str
> + #: The names of virtual devices to test.
> + vdevs: list[str] = Field(default_factory=list)
> #: The seed to use for pseudo-random generation.
> random_seed: int | None = None
>
> @@ -464,12 +465,12 @@ def test_runs_with_nodes(self) ->
> list[TestRunWithNodesConfiguration]:
> test_runs_with_nodes = []
>
> for test_run_no, test_run in enumerate(self.test_runs):
> - sut_node_name = test_run.system_under_test_node.node_name
> + sut_node_name = test_run.system_under_test_node
> sut_node = next(filter(lambda n: n.name == sut_node_name,
> self.nodes), None)
>
> assert sut_node is not None, (
> f"test_runs.{test_run_no}.sut_node_config.node_name "
> - f"({test_run.system_under_test_node.node_name}) is not a
> valid node name"
> + f"({test_run.system_under_test_node}) is not a valid node
> name"
> )
> assert isinstance(sut_node, SutNodeConfiguration), (
> f"test_runs.{test_run_no}.sut_node_config.node_name is a
> valid node name, "
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 510be1a870..0cdbb07e06 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -277,7 +277,7 @@ def _connect_nodes_and_run_test_run(
> tg_node = TGNode(tg_node_config)
> tg_nodes[tg_node.name] = tg_node
> except Exception as e:
> - failed_node = test_run_config.system_under_test_node.node_name
> + failed_node = test_run_config.system_under_test_node
> if sut_node:
> failed_node = test_run_config.traffic_generator_node
> self._logger.exception(f"The Creation of node {failed_node}
> failed.")
> @@ -315,9 +315,7 @@ def _run_test_run(
> Raises:
> ConfigurationError: If the DPDK sources or build is not set up
> from config or settings.
> """
> - self._logger.info(
> - f"Running test run with SUT
> '{test_run_config.system_under_test_node.node_name}'."
> - )
> + self._logger.info(f"Running test run with SUT
> '{test_run_config.system_under_test_node}'.")
> test_run_result.ports = sut_node.ports
> test_run_result.sut_info = sut_node.node_info
> try:
> diff --git a/dts/framework/testbed_model/cpu.py
> b/dts/framework/testbed_model/cpu.py
> index d19fa5d597..b8bc601c22 100644
> --- a/dts/framework/testbed_model/cpu.py
> +++ b/dts/framework/testbed_model/cpu.py
> @@ -185,7 +185,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]:
> @@ -228,6 +227,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:
> @@ -356,6 +357,9 @@ def filter(self) -> list[LogicalCore]:
> Raises:
> ValueError: If the specified lcore filter specifier is invalid.
> """
> + 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 e3732f0827..41e7656b0f 100644
> --- a/dts/framework/testbed_model/linux_session.py
> +++ b/dts/framework/testbed_model/linux_session.py
> @@ -67,15 +67,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 b08b1cf14d..6c2dfd6185 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -24,14 +24,7 @@
> from framework.exception import ConfigurationError
> from framework.logger import DTSLogger, get_dts_logger
>
> -from .cpu import (
> - Architecture,
> - LogicalCore,
> - LogicalCoreCount,
> - LogicalCoreList,
> - LogicalCoreListFilter,
> - lcore_filter,
> -)
> +from .cpu import Architecture, LogicalCore, LogicalCoreCount,
> LogicalCoreList, lcore_filter
> from .linux_session import LinuxSession
> from .os_session import OSSession
> from .port import Port
> @@ -82,24 +75,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()
>
> @@ -188,7 +165,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 fcda9b3de1..e436886692 100644
> --- a/dts/framework/testbed_model/os_session.py
> +++ b/dts/framework/testbed_model/os_session.py
> @@ -445,7 +445,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 11d4b22089..d8f1f9d452 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -33,6 +33,7 @@
> from framework.remote_session.remote_session import CommandResult
> from framework.utils import MesonArgs, TarCompressionFormat
>
> +from .cpu import LogicalCore, LogicalCoreList
> from .node import Node
> from .os_session import OSSession, OSSessionInfo
> from .virtual_device import VirtualDevice
> @@ -92,6 +93,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._env_vars = {}
> @@ -198,7 +210,7 @@ def set_up_test_run(
> dpdk_build_config: The build configuration of DPDK.
> """
> super().set_up_test_run(test_run_config, dpdk_build_config)
> - for vdev in test_run_config.system_under_test_node.vdevs:
> + for vdev in test_run_config.vdevs:
> self.virtual_devices.append(VirtualDevice(vdev))
> self._set_up_dpdk(dpdk_build_config)
>
> --
> 2.43.0
>
