Re: [PATCH v2 2/6] dts: add traffic generator config

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

[-- Attachment #1: Type: text/plain, Size: 16003 bytes --]

Hey Juraj,

These changes look good to me, I just had one question. Is the plan to make
specifying a TG always required or optional for cases where it isn't
required? It seems like it is written now as something that is required for
every execution. I don't think this is necessarily a bad thing, I just
wonder if it would be beneficial at all to make it optional to allow for
executions that don't utilize a TG to just not specify it.

On Mon, Jul 17, 2023 at 7:07 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> Node configuration - where to connect, what ports to use and what TG to
> use.
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  dts/conf.yaml                              | 26 ++++++-
>  dts/framework/config/__init__.py           | 87 +++++++++++++++++++---
>  dts/framework/config/conf_yaml_schema.json | 29 +++++++-
>  dts/framework/dts.py                       | 12 +--
>  dts/framework/testbed_model/node.py        |  4 +-
>  dts/framework/testbed_model/sut_node.py    |  6 +-
>  6 files changed, 141 insertions(+), 23 deletions(-)
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 3a5d87cb49..7f089022ba 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -13,10 +13,11 @@ executions:
>      skip_smoke_tests: false # optional flag that allow you to skip smoke
> tests
>      test_suites:
>        - hello_world
> -    system_under_test:
> +    system_under_test_node:
>        node_name: "SUT 1"
>        vdevs: # optional; if removed, vdevs won't be used in the execution
>          - "crypto_openssl"
> +    traffic_generator_node: "TG 1"
>  nodes:
>    - name: "SUT 1"
>      hostname: sut1.change.me.localhost
> @@ -40,3 +41,26 @@ nodes:
>          os_driver: i40e
>          peer_node: "TG 1"
>          peer_pci: "0000:00:08.1"
> +  - name: "TG 1"
> +    hostname: tg1.change.me.localhost
> +    user: dtsuser
> +    arch: x86_64
> +    os: linux
> +    lcores: ""
> +    ports:
> +      - pci: "0000:00:08.0"
> +        os_driver_for_dpdk: rdma
> +        os_driver: rdma
> +        peer_node: "SUT 1"
> +        peer_pci: "0000:00:08.0"
> +      - pci: "0000:00:08.1"
> +        os_driver_for_dpdk: rdma
> +        os_driver: rdma
> +        peer_node: "SUT 1"
> +        peer_pci: "0000:00:08.1"
> +    use_first_core: false
> +    hugepages:  # optional; if removed, will use system hugepage
> configuration
> +        amount: 256
> +        force_first_numa: false
> +    traffic_generator:
> +        type: SCAPY
> diff --git a/dts/framework/config/__init__.py
> b/dts/framework/config/__init__.py
> index fad56cc520..72aa021b97 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -13,7 +13,7 @@
>  from dataclasses import dataclass
>  from enum import Enum, auto, unique
>  from pathlib import PurePath
> -from typing import Any, TypedDict
> +from typing import Any, TypedDict, Union
>
>  import warlock  # type: ignore
>  import yaml
> @@ -55,6 +55,11 @@ class Compiler(StrEnum):
>      msvc = auto()
>
>
> +@unique
> +class TrafficGeneratorType(StrEnum):
> +    SCAPY = auto()
> +
> +
>  # Slots enables some optimizations, by pre-allocating space for the
> defined
>  # attributes in the underlying data structure.
>  #
> @@ -79,6 +84,29 @@ class PortConfig:
>      def from_dict(node: str, d: dict) -> "PortConfig":
>          return PortConfig(node=node, **d)
>
> +    def __str__(self) -> str:
> +        return f"Port {self.pci} on node {self.node}"
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class TrafficGeneratorConfig:
> +    traffic_generator_type: TrafficGeneratorType
> +
> +    @staticmethod
> +    def from_dict(d: dict):
> +        # This looks useless now, but is designed to allow expansion to
> traffic
> +        # generators that require more configuration later.
> +        match TrafficGeneratorType(d["type"]):
> +            case TrafficGeneratorType.SCAPY:
> +                return ScapyTrafficGeneratorConfig(
> +                    traffic_generator_type=TrafficGeneratorType.SCAPY
> +                )
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class ScapyTrafficGeneratorConfig(TrafficGeneratorConfig):
> +    pass
> +
>
>  @dataclass(slots=True, frozen=True)
>  class NodeConfiguration:
> @@ -90,17 +118,17 @@ class NodeConfiguration:
>      os: OS
>      lcores: str
>      use_first_core: bool
> -    memory_channels: int
>      hugepages: HugepageConfiguration | None
>      ports: list[PortConfig]
>
>      @staticmethod
> -    def from_dict(d: dict) -> "NodeConfiguration":
> +    def from_dict(d: dict) -> Union["SutNodeConfiguration",
> "TGNodeConfiguration"]:
>          hugepage_config = d.get("hugepages")
>          if hugepage_config:
>              if "force_first_numa" not in hugepage_config:
>                  hugepage_config["force_first_numa"] = False
>              hugepage_config = HugepageConfiguration(**hugepage_config)
> +
>          common_config = {
>              "name": d["name"],
>              "hostname": d["hostname"],
> @@ -110,12 +138,31 @@ def from_dict(d: dict) -> "NodeConfiguration":
>              "os": OS(d["os"]),
>              "lcores": d.get("lcores", "1"),
>              "use_first_core": d.get("use_first_core", False),
> -            "memory_channels": d.get("memory_channels", 1),
>              "hugepages": hugepage_config,
>              "ports": [PortConfig.from_dict(d["name"], port) for port in
> d["ports"]],
>          }
>
> -        return NodeConfiguration(**common_config)
> +        if "traffic_generator" in d:
> +            return TGNodeConfiguration(
> +                traffic_generator=TrafficGeneratorConfig.from_dict(
> +                    d["traffic_generator"]
> +                ),
> +                **common_config,
> +            )
> +        else:
> +            return SutNodeConfiguration(
> +                memory_channels=d.get("memory_channels", 1),
> **common_config
> +            )
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class SutNodeConfiguration(NodeConfiguration):
> +    memory_channels: int
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class TGNodeConfiguration(NodeConfiguration):
> +    traffic_generator: ScapyTrafficGeneratorConfig
>
>
>  @dataclass(slots=True, frozen=True)
> @@ -194,23 +241,40 @@ class ExecutionConfiguration:
>      perf: bool
>      func: bool
>      test_suites: list[TestSuiteConfig]
> -    system_under_test: NodeConfiguration
> +    system_under_test_node: SutNodeConfiguration
> +    traffic_generator_node: TGNodeConfiguration
>      vdevs: list[str]
>      skip_smoke_tests: bool
>
>      @staticmethod
> -    def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
> +    def from_dict(
> +        d: dict, node_map: dict[str, Union[SutNodeConfiguration |
> TGNodeConfiguration]]
> +    ) -> "ExecutionConfiguration":
>          build_targets: list[BuildTargetConfiguration] = list(
>              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_name"]
> +        sut_name = d["system_under_test_node"]["node_name"]
>          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]
> +        assert isinstance(
> +            system_under_test_node, SutNodeConfiguration
> +        ), f"Invalid SUT configuration {system_under_test_node}"
> +
> +        tg_name = d["traffic_generator_node"]
> +        assert tg_name in node_map, f"Unknown TG {tg_name} in execution
> {d}"
> +        traffic_generator_node = node_map[tg_name]
> +        assert isinstance(
> +            traffic_generator_node, TGNodeConfiguration
> +        ), f"Invalid TG configuration {traffic_generator_node}"
> +
>
         vdevs = (
> -            d["system_under_test"]["vdevs"] if "vdevs" in
> d["system_under_test"] else []
> +            d["system_under_test_node"]["vdevs"]
> +            if "vdevs" in d["system_under_test_node"]
> +            else []
>          )
>          return ExecutionConfiguration(
>              build_targets=build_targets,
> @@ -218,7 +282,8 @@ def from_dict(d: dict, node_map: dict) ->
> "ExecutionConfiguration":
>              func=d["func"],
>              skip_smoke_tests=skip_smoke_tests,
>              test_suites=test_suites,
> -            system_under_test=node_map[sut_name],
> +            system_under_test_node=system_under_test_node,
> +            traffic_generator_node=traffic_generator_node,
>              vdevs=vdevs,
>          )
>
> @@ -229,7 +294,7 @@ class Configuration:
>
>      @staticmethod
>      def from_dict(d: dict) -> "Configuration":
> -        nodes: list[NodeConfiguration] = list(
> +        nodes: list[Union[SutNodeConfiguration | TGNodeConfiguration]] =
> list(
>              map(NodeConfiguration.from_dict, d["nodes"])
>          )
>          assert len(nodes) > 0, "There must be a node to test"
> diff --git a/dts/framework/config/conf_yaml_schema.json
> b/dts/framework/config/conf_yaml_schema.json
> index 61f52b4365..76df84840a 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -164,6 +164,11 @@
>          "amount"
>        ]
>      },
> +    "mac_address": {
> +      "type": "string",
> +      "description": "A MAC address",
> +      "pattern": "^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$"
> +    },
>      "pci_address": {
>        "type": "string",
>        "pattern":
> "^[\\da-fA-F]{4}:[\\da-fA-F]{2}:[\\da-fA-F]{2}.\\d:?\\w*$"
> @@ -286,6 +291,22 @@
>                ]
>              },
>              "minimum": 1
> +          },
> +          "traffic_generator": {
> +            "oneOf": [
> +              {
> +                "type": "object",
> +                "description": "Scapy traffic generator. Used for
> functional testing.",
> +                "properties": {
> +                  "type": {
> +                    "type": "string",
> +                    "enum": [
> +                      "SCAPY"
> +                    ]
> +                  }
> +                }
> +              }
> +            ]
>            }
>          },
>          "additionalProperties": false,
> @@ -336,7 +357,7 @@
>              "description": "Optional field that allows you to skip smoke
> testing",
>              "type": "boolean"
>            },
> -          "system_under_test": {
> +          "system_under_test_node": {
>              "type":"object",
>              "properties": {
>                "node_name": {
> @@ -353,6 +374,9 @@
>              "required": [
>                "node_name"
>              ]
> +          },
> +          "traffic_generator_node": {
> +            "$ref": "#/definitions/node_name"
>            }
>          },
>          "additionalProperties": false,
> @@ -361,7 +385,8 @@
>            "perf",
>            "func",
>            "test_suites",
> -          "system_under_test"
> +          "system_under_test_node",
> +          "traffic_generator_node"
>          ]
>        },
>        "minimum": 1
> diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> index 7b09d8fba8..372bc72787 100644
> --- a/dts/framework/dts.py
> +++ b/dts/framework/dts.py
> @@ -38,17 +38,17 @@ def run_all() -> None:
>          # for all Execution sections
>          for execution in CONFIGURATION.executions:
>              sut_node = None
> -            if execution.system_under_test.name in nodes:
> +            if execution.system_under_test_node.name in nodes:
>                  # a Node with the same name already exists
> -                sut_node = nodes[execution.system_under_test.name]
> +                sut_node = nodes[execution.system_under_test_node.name]
>              else:
>                  # the SUT has not been initialized yet
>                  try:
> -                    sut_node = SutNode(execution.system_under_test)
> +                    sut_node = SutNode(execution.system_under_test_node)
>                      result.update_setup(Result.PASS)
>                  except Exception as e:
>                      dts_logger.exception(
> -                        f"Connection to node
> {execution.system_under_test} failed."
> +                        f"Connection to node
> {execution.system_under_test_node} failed."
>                      )
>                      result.update_setup(Result.FAIL, e)
>                  else:
> @@ -87,7 +87,9 @@ def _run_execution(
>      Run the given execution. This involves running the execution setup as
> well as
>      running all build targets in the given execution.
>      """
> -    dts_logger.info(f"Running execution with SUT '{
> execution.system_under_test.name}'.")
> +    dts_logger.info(
> +        f"Running execution with SUT '{
> execution.system_under_test_node.name}'."
> +    )
>      execution_result = result.add_execution(sut_node.config,
> sut_node.node_info)
>
>      try:
> diff --git a/dts/framework/testbed_model/node.py
> b/dts/framework/testbed_model/node.py
> index c5147e0ee6..d2d55d904e 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -48,6 +48,8 @@ def __init__(self, node_config: NodeConfiguration):
>          self._logger = getLogger(self.name)
>          self.main_session = create_session(self.config, self.name,
> self._logger)
>
> +        self._logger.info(f"Connected to node: {self.name}")
> +
>          self._get_remote_cpus()
>          # filter the node lcores according to user config
>          self.lcores = LogicalCoreListFilter(
> @@ -56,8 +58,6 @@ def __init__(self, node_config: NodeConfiguration):
>
>          self._other_sessions = []
>
> -        self._logger.info(f"Created node: {self.name}")
> -
>      def set_up_execution(self, execution_config: ExecutionConfiguration)
> -> None:
>          """
>          Perform the execution setup that will be done for each execution
> diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> index 53953718a1..bcad364435 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -13,8 +13,8 @@
>      BuildTargetConfiguration,
>      BuildTargetInfo,
>      InteractiveApp,
> -    NodeConfiguration,
>      NodeInfo,
> +    SutNodeConfiguration,
>  )
>  from framework.remote_session import (
>      CommandResult,
> @@ -83,6 +83,7 @@ class SutNode(Node):
>      Another key capability is building DPDK according to given build
> target.
>      """
>
> +    config: SutNodeConfiguration
>      _dpdk_prefix_list: list[str]
>      _dpdk_timestamp: str
>      _build_target_config: BuildTargetConfiguration | None
> @@ -95,7 +96,7 @@ class SutNode(Node):
>      _node_info: NodeInfo | None
>      _compiler_version: str | None
>
> -    def __init__(self, node_config: NodeConfiguration):
> +    def __init__(self, node_config: SutNodeConfiguration):
>          super(SutNode, self).__init__(node_config)
>          self._dpdk_prefix_list = []
>          self._build_target_config = None
> @@ -110,6 +111,7 @@ def __init__(self, node_config: NodeConfiguration):
>          self._dpdk_version = None
>          self._node_info = None
>          self._compiler_version = None
> +        self._logger.info(f"Created node: {self.name}")
>
>      @property
>      def _remote_dpdk_dir(self) -> PurePath:
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 21341 bytes --]