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

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

On Tue, Jul 18, 2023 at 5:55 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> 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.
>

If there's a legitimate use case (or we can imagine one) for this in
CI (or anywhere else), then we'd want to make a TG node optional.
Let's leave the implementation as is and make the TG node optional
once we discuss it further in the CI call. We talked about a mechanism
to skip test cases based on the testbed capabilities and I think this
would fit into that (as TG could be viewed as a testbed capability).

> 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
>>