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

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

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

On Wed, Jul 19, 2023, 08:57 Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:

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

I like that idea of thinking of it as a testbed capability and skipping
based on that. I think that would work nicely to address this if there was
a use case for it. Thanks for the input!


> > 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: 25944 bytes --]