Re: [PATCH v2 1/2] dts: add smoke tests

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

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

On Tue, Jul 11, 2023 at 5:42 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> Just a few more comments.
>
> On Mon, Jul 10, 2023 at 6:23 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Adds a new test suite for running smoke tests that verify general
> > configuration aspects of the system under test. If any of these tests
> > fail, the DTS execution terminates as part of a "fail-fast" model.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> >  dts/conf.yaml                                 |  17 +-
> >  dts/framework/config/__init__.py              | 116 +++++++++--
> >  dts/framework/config/conf_yaml_schema.json    | 142 +++++++++++++-
> >  dts/framework/dts.py                          |  88 ++++++---
> >  dts/framework/exception.py                    |  12 ++
> >  dts/framework/remote_session/__init__.py      |  10 +-
> >  dts/framework/remote_session/os_session.py    |  24 ++-
> >  dts/framework/remote_session/posix_session.py |  29 ++-
> >  .../remote_session/remote/__init__.py         |  10 +
> >  .../remote/interactive_remote_session.py      | 118 ++++++++++++
> >  .../remote/interactive_shell.py               |  99 ++++++++++
> >  .../remote_session/remote/testpmd_shell.py    |  67 +++++++
> >  dts/framework/test_result.py                  |  37 +++-
> >  dts/framework/test_suite.py                   |  21 +-
> >  dts/framework/testbed_model/node.py           |   2 +
> >  dts/framework/testbed_model/sut_node.py       | 180 +++++++++++++-----
> >  dts/tests/TestSuite_smoke_tests.py            | 118 ++++++++++++
> >  17 files changed, 994 insertions(+), 96 deletions(-)
> >  create mode 100644
> dts/framework/remote_session/remote/interactive_remote_session.py
> >  create mode 100644
> dts/framework/remote_session/remote/interactive_shell.py
> >  create mode 100644 dts/framework/remote_session/remote/testpmd_shell.py
> >  create mode 100644 dts/tests/TestSuite_smoke_tests.py
> >
> > diff --git a/dts/conf.yaml b/dts/conf.yaml
> > index a9bd8a3e..2717de13 100644
> > --- a/dts/conf.yaml
> > +++ b/dts/conf.yaml
> > @@ -10,9 +10,13 @@ executions:
> >          compiler_wrapper: ccache
> >      perf: false
> >      func: true
> > +    skip_smoke_tests: false #optional flag that allow you to ski smoke
> tests
>
> Typo: ski
> Also put a space after #
>
> >      test_suites:
> >        - hello_world
> > -    system_under_test: "SUT 1"
> > +    system_under_test:
> > +      node_name: "SUT 1"
> > +      vdevs: #optional: if removed vdevs won't be used in the execution
>
> Missing space after #
> The sentence after hugepages has a comma in it, let's unify those.
>
> > +        - "crypto_openssl"
> >  nodes:
> >    - name: "SUT 1"
> >      hostname: sut1.change.me.localhost
> > @@ -20,6 +24,17 @@ nodes:
> >      arch: x86_64
> >      os: linux
> >      lcores: ""
> > +    ports:
>
> I'm comparing my version with this patch and I've just noticed this -
> let's put the ports at the end (after hugepages). This way we'll have
> the configuration sorted into sections of sorts:
> Cores/cpu config
> Memory config
> Port/devices config
>
> > +      - pci: "0000:00:08.0"
> > +        os_driver_for_dpdk: vfio-pci #OS driver that DPDK will use
>
> Missing space after #
>
>
Sorry, I'm just used to writing comments without the space but I'll fix
this.



> > +        os_driver: i40e
> > +        peer_node: "TG 1"
> > +        peer_pci: "0000:00:08.0"
> > +      - pci: "0000:00:08.1"
> > +        os_driver_for_dpdk: vfio-pci
> > +        os_driver: i40e
> > +        peer_node: "TG 1"
> > +        peer_pci: "0000:00:08.1"
> >      use_first_core: false
> >      memory_channels: 4
> >      hugepages:  # optional; if removed, will use system hugepage
> configuration
> > diff --git a/dts/framework/config/__init__.py
> b/dts/framework/config/__init__.py
> > index ebb0823f..75ac1cbe 100644
> > --- a/dts/framework/config/__init__.py
> > +++ b/dts/framework/config/__init__.py
> > @@ -12,6 +12,7 @@
> >  import pathlib
> >  from dataclasses import dataclass
> >  from enum import Enum, auto, unique
> > +from pathlib import PurePath
> >  from typing import Any, TypedDict
> >
> >  import warlock  # type: ignore
> > @@ -72,6 +73,20 @@ class HugepageConfiguration:
> >      force_first_numa: bool
> >
> >
> > +@dataclass(slots=True, frozen=True)
> > +class PortConfig:
> > +    node: str
> > +    pci: str
> > +    os_driver_for_dpdk: str
> > +    os_driver: str
> > +    peer_node: str
> > +    peer_pci: str
> > +
> > +    @staticmethod
> > +    def from_dict(node: str, d: dict) -> "PortConfig":
> > +        return PortConfig(node=node, **d)
> > +
> > +
> >  @dataclass(slots=True, frozen=True)
> >  class NodeConfiguration:
> >      name: str
> > @@ -84,6 +99,7 @@ class NodeConfiguration:
> >      use_first_core: bool
> >      memory_channels: int
> >      hugepages: HugepageConfiguration | None
> > +    ports: list[PortConfig]
> >
> >      @staticmethod
> >      def from_dict(d: dict) -> "NodeConfiguration":
> > @@ -92,18 +108,43 @@ def from_dict(d: dict) -> "NodeConfiguration":
> >              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"],
> > +            "user": d["user"],
> > +            "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),
> > +            "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)
> > +
> > +
> > +@dataclass(slots=True)
>
> Looks like this could be frozen as well. This should work even in the
> future, as I imagine we'll get all the info we need just once.
>

You're right, this data isn't really something that's going to change once
we collect it and it's probably better to enforce that. I'll make this
change.


> > +class NodeInfo:
> > +    """Class to hold important versions within the node.
> > +
> > +    This class, unlike the NodeConfiguration class, cannot be generated
> at the start.
> > +    This is because we need to initialize a connection with the node
> before we can
> > +    collect the information needed in this class. Therefore, it cannot
> be a part of
> > +    the configuration class above.
> > +    """
> >
> > -        return NodeConfiguration(
> > -            name=d["name"],
> > -            hostname=d["hostname"],
> > -            user=d["user"],
> > -            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),
> > -            memory_channels=d.get("memory_channels", 1),
> > -            hugepages=hugepage_config,
> > +    os_name: str
> > +    os_version: str
> > +    kernel_version: str
> > +
> > +    @staticmethod
> > +    def from_dict(d: dict):
> > +        return NodeInfo(
> > +            os_name=d["os_name"],
> > +            os_version=d["os_version"],
> > +            kernel_version=d["kernel_version"],
> >          )
>
> We don't need the from_dict method, as we can instantiate this class
> right away (node_info = NodeInfo(os_name=d["os_name"],
> os_version=d["os_version"], kernel_version=d["kernel_version"])).
>
> The other classes need this method because we're doing some processing
> before instantiating the classes - this one doesn't need it.
>

Good point, I included it to make it similar to the others but it really
isn't necessary because I'm not even using it.


>
> >
> >
> > @@ -128,6 +169,24 @@ def from_dict(d: dict) ->
> "BuildTargetConfiguration":
> >          )
> >
> >
> > +@dataclass(slots=True)
>
> Can also be frozen.
>
> > +class BuildTargetInfo:
> > +    """Class to hold important versions within the build target.
> > +
> > +    This is very similar to the NodeVersionInfo class, it just instead
> holds information
>
> References renamed class.
>

Good catch, I totally missed the reference to the other class in this
comment.


>
> > +    for the build target.
> > +    """
> > +
> > +    dpdk_version: str
> > +    compiler_version: str
> > +
> > +    @staticmethod
> > +    def from_dict(d: dict):
> > +        return BuildTargetInfo(
> > +            dpdk_version=d["dpdk_version"],
> compiler_version=d["compiler_version"]
> > +        )
>
> Same as above.
>

Good catches on these as well, I'll be sure to make the changes.


>
> > +
> > +
> >  class TestSuiteConfigDict(TypedDict):
> >      suite: str
> >      cases: list[str]
> > @@ -157,6 +216,8 @@ class ExecutionConfiguration:
> >      func: bool
> >      test_suites: list[TestSuiteConfig]
> >      system_under_test: NodeConfiguration
> > +    vdevs: list[str]
> > +    skip_smoke_tests: bool
> >
> >      @staticmethod
> >      def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
> > @@ -166,15 +227,20 @@ def from_dict(d: dict, node_map: dict) ->
> "ExecutionConfiguration":
> >          test_suites: list[TestSuiteConfig] = list(
> >              map(TestSuiteConfig.from_dict, d["test_suites"])
> >          )
> > -        sut_name = d["system_under_test"]
> > +        sut_name = d["system_under_test"]["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}"
> > -
> > +        vdevs = (
> > +            d["system_under_test"]["vdevs"] if "vdevs" in
> d["system_under_test"] else []
> > +        )
> >          return ExecutionConfiguration(
> >              build_targets=build_targets,
> >              perf=d["perf"],
> >              func=d["func"],
> > +            skip_smoke_tests=skip_smoke_tests,
> >              test_suites=test_suites,
> >              system_under_test=node_map[sut_name],
> > +            vdevs=vdevs,
> >          )
> >
> >
> > @@ -221,3 +287,27 @@ def load_config() -> Configuration:
> >
> >
> >  CONFIGURATION = load_config()
> > +
> > +
> > +@unique
> > +class InteractiveApp(Enum):
> > +    """An enum that represents different supported interactive
> applications
> > +
> > +    The values in this enum must all be set to objects that have a key
> called
> > +    "default_path" where "default_path" represents a PurPath object for
> the path
> > +    to the application. This default path will be passed into the
> handler class
> > +    for the application so that it can start the application. For every
> key other
> > +    than the default shell option, the path will be appended to the
> path to the DPDK
> > +    build directory for the current SUT node.
> > +    """
> > +
> > +    shell = {"default_path": PurePath()}
> > +    testpmd = {"default_path": PurePath("app", "dpdk-testpmd")}
> > +
> > +    def get_path(self) -> PurePath:
> > +        """A method for getting the default paths of an application
> > +
> > +        Returns:
> > +            String array that represents an OS agnostic path to the
> application.
> > +        """
> > +        return self.value["default_path"]
> > diff --git a/dts/framework/config/conf_yaml_schema.json
> b/dts/framework/config/conf_yaml_schema.json
> > index ca2d4a1e..61f52b43 100644
> > --- a/dts/framework/config/conf_yaml_schema.json
> > +++ b/dts/framework/config/conf_yaml_schema.json
> > @@ -6,6 +6,76 @@
> >        "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": [
> > @@ -94,6 +164,19 @@
> >          "amount"
> >        ]
> >      },
> > +    "pci_address": {
> > +      "type": "string",
> > +      "pattern":
> "^[\\da-fA-F]{4}:[\\da-fA-F]{2}:[\\da-fA-F]{2}.\\d:?\\w*$"
> > +    },
> > +    "port_peer_address": {
> > +      "description": "Peer is a TRex port, and IXIA port or a PCI
> address",
> > +      "oneOf": [
> > +        {
> > +          "description": "PCI peer port",
> > +          "$ref": "#/definitions/pci_address"
> > +        }
> > +      ]
> > +    },
> >      "test_suite": {
> >        "type": "string",
> >        "enum": [
> > @@ -165,6 +248,44 @@
> >            },
> >            "hugepages": {
> >              "$ref": "#/definitions/hugepages"
> > +          },
> > +          "ports": {
> > +            "type": "array",
> > +            "items": {
> > +              "type": "object",
> > +              "description": "Each port should be described on both
> sides of the connection. This makes configuration slightly more verbose but
> greatly simplifies implementation. If there are an inconsistencies, then
> DTS will not run until that issue is fixed. An example inconsistency would
> be port 1, node 1 says it is connected to port 1, node 2, but port 1, node
> 2 says it is connected to port 2, node 1.",
> > +              "properties": {
> > +                "pci": {
> > +                  "$ref": "#/definitions/pci_address",
> > +                  "description": "The local PCI address of the port"
> > +                },
> > +                "os_driver_for_dpdk": {
> > +                  "type": "string",
> > +                  "description": "The driver that the kernel should
> bind this device to for DPDK to use it. (ex: vfio-pci)"
> > +                },
> > +                "os_driver": {
> > +                  "type": "string",
> > +                  "description": "The driver normally used by this port
> (ex: i40e)"
> > +                },
> > +                "peer_node": {
> > +                  "type": "string",
> > +                  "description": "The name of the node the peer port is
> on"
> > +                },
> > +                "peer_pci": {
> > +                  "$ref": "#/definitions/pci_address",
> > +                  "description": "The PCI address of the peer port"
> > +                }
> > +              },
> > +              "additionalProperties": false,
> > +              "required": [
> > +                "pci",
> > +                "os_driver_for_dpdk",
> > +                "os_driver",
> > +                "peer_node",
> > +                "peer_pci"
> > +              ]
> > +            },
> > +            "minimum": 1
> >            }
> >          },
> >          "additionalProperties": false,
> > @@ -211,8 +332,27 @@
> >                ]
> >              }
> >            },
> > +          "skip_smoke_tests": {
> > +            "description": "Optional field that allows you to skip
> smoke testing",
> > +            "type": "boolean"
> > +          },
> >            "system_under_test": {
> > -            "$ref": "#/definitions/node_name"
> > +            "type":"object",
> > +            "properties": {
> > +              "node_name": {
> > +                "$ref": "#/definitions/node_name"
> > +              },
> > +              "vdevs": {
> > +                "description": "Opentional list of names of vdevs to be
> used in execution",
> > +                "type": "array",
> > +                "items": {
> > +                  "type": "string"
> > +                }
> > +              }
> > +            },
> > +            "required": [
> > +              "node_name"
> > +            ]
> >            }
> >          },
> >          "additionalProperties": false,
> > diff --git a/dts/framework/dts.py b/dts/framework/dts.py
> > index 05022845..1b67938f 100644
> > --- a/dts/framework/dts.py
> > +++ b/dts/framework/dts.py
> > @@ -5,7 +5,13 @@
> >
> >  import sys
> >
> > -from .config import CONFIGURATION, BuildTargetConfiguration,
> ExecutionConfiguration
> > +from .config import (
> > +    CONFIGURATION,
> > +    BuildTargetConfiguration,
> > +    ExecutionConfiguration,
> > +    TestSuiteConfig,
> > +)
> > +from .exception import BlockingTestSuiteError
> >  from .logger import DTSLOG, getLogger
> >  from .test_result import BuildTargetResult, DTSResult, ExecutionResult,
> Result
> >  from .test_suite import get_test_suites
> > @@ -82,7 +88,7 @@ def _run_execution(
> >      running all build targets in the given execution.
> >      """
> >      dts_logger.info(f"Running execution with SUT '{
> execution.system_under_test.name}'.")
> > -    execution_result = result.add_execution(sut_node.config)
> > +    execution_result = result.add_execution(sut_node.config,
> sut_node.node_info)
> >
> >      try:
> >          sut_node.set_up_execution(execution)
> > @@ -118,14 +124,15 @@ def _run_build_target(
> >
> >      try:
> >          sut_node.set_up_build_target(build_target)
> > -        result.dpdk_version = sut_node.dpdk_version
> > +        # result.dpdk_version = sut_node.dpdk_version
> > +
> build_target_result.add_build_target_versions(sut_node.get_build_target_info())
> >          build_target_result.update_setup(Result.PASS)
> >      except Exception as e:
> >          dts_logger.exception("Build target setup failed.")
> >          build_target_result.update_setup(Result.FAIL, e)
> >
> >      else:
> > -        _run_suites(sut_node, execution, build_target_result)
> > +        _run_all_suites(sut_node, execution, build_target_result)
> >
> >      finally:
> >          try:
> > @@ -136,7 +143,7 @@ def _run_build_target(
> >              build_target_result.update_teardown(Result.FAIL, e)
> >
> >
> > -def _run_suites(
> > +def _run_all_suites(
> >      sut_node: SutNode,
> >      execution: ExecutionConfiguration,
> >      build_target_result: BuildTargetResult,
> > @@ -146,27 +153,62 @@ def _run_suites(
> >      with possibly only a subset of test cases.
> >      If no subset is specified, run all test cases.
> >      """
> > +    end_build_target = False
> > +    if not execution.skip_smoke_tests:
> > +        execution.test_suites[:0] =
> [TestSuiteConfig.from_dict("smoke_tests")]
> >      for test_suite_config in execution.test_suites:
> >          try:
> > -            full_suite_path =
> f"tests.TestSuite_{test_suite_config.test_suite}"
> > -            test_suite_classes = get_test_suites(full_suite_path)
> > -            suites_str = ", ".join((x.__name__ for x in
> test_suite_classes))
> > -            dts_logger.debug(
> > -                f"Found test suites '{suites_str}' in
> '{full_suite_path}'."
> > +            _run_single_suite(
> > +                sut_node, execution, build_target_result,
> test_suite_config
> >              )
> > -        except Exception as e:
> > -            dts_logger.exception("An error occurred when searching for
> test suites.")
> > -            result.update_setup(Result.ERROR, e)
> > -
> > -        else:
> > -            for test_suite_class in test_suite_classes:
> > -                test_suite = test_suite_class(
> > -                    sut_node,
> > -                    test_suite_config.test_cases,
> > -                    execution.func,
> > -                    build_target_result,
> > -                )
> > -                test_suite.run()
> > +        except BlockingTestSuiteError as e:
> > +            dts_logger.exception(
> > +                f"An error occurred within
> {test_suite_config.test_suite}. "
> > +                "Skipping build target..."
> > +            )
> > +            result.add_error(e)
> > +            end_build_target = True
> > +        # if a blocking test failed and we need to bail out of suite
> executions
> > +        if end_build_target:
> > +            break
> > +
> > +
> > +def _run_single_suite(
> > +    sut_node: SutNode,
> > +    execution: ExecutionConfiguration,
> > +    build_target_result: BuildTargetResult,
> > +    test_suite_config: TestSuiteConfig,
> > +) -> None:
> > +    """Runs a single test suite.
> > +
> > +    Args:
> > +        sut_node: Node to run tests on.
> > +        execution: Execution the test case belongs to.
> > +        build_target_result: Build target configuration test case is
> run on
> > +        test_suite_config: Test suite configuration
> > +
> > +    Raises:
> > +        BlockingTestSuiteError: If a test suite that was marked as
> blocking fails.
> > +    """
> > +    try:
> > +        full_suite_path =
> f"tests.TestSuite_{test_suite_config.test_suite}"
> > +        test_suite_classes = get_test_suites(full_suite_path)
> > +        suites_str = ", ".join((x.__name__ for x in test_suite_classes))
> > +        dts_logger.debug(f"Found test suites '{suites_str}' in
> '{full_suite_path}'.")
> > +    except Exception as e:
> > +        dts_logger.exception("An error occurred when searching for test
> suites.")
> > +        result.update_setup(Result.ERROR, e)
> > +
> > +    else:
> > +        for test_suite_class in test_suite_classes:
> > +            test_suite = test_suite_class(
> > +                sut_node,
> > +                test_suite_config.test_cases,
> > +                execution.func,
> > +                build_target_result,
> > +                result,
> > +            )
> > +            test_suite.run()
> >
> >
> >  def _exit_dts() -> None:
> > diff --git a/dts/framework/exception.py b/dts/framework/exception.py
> > index ca353d98..dfb12df4 100644
> > --- a/dts/framework/exception.py
> > +++ b/dts/framework/exception.py
> > @@ -25,6 +25,7 @@ class ErrorSeverity(IntEnum):
> >      SSH_ERR = 4
> >      DPDK_BUILD_ERR = 10
> >      TESTCASE_VERIFY_ERR = 20
> > +    BLOCKING_TESTSUITE_ERR = 25
> >
> >
> >  class DTSError(Exception):
> > @@ -144,3 +145,14 @@ def __init__(self, value: str):
> >
> >      def __str__(self) -> str:
> >          return repr(self.value)
> > +
> > +
> > +class BlockingTestSuiteError(DTSError):
> > +    suite_name: str
> > +    severity: ClassVar[ErrorSeverity] =
> ErrorSeverity.BLOCKING_TESTSUITE_ERR
> > +
> > +    def __init__(self, suite_name: str) -> None:
> > +        self.suite_name = suite_name
> > +
> > +    def __str__(self) -> str:
> > +        return f"Blocking suite {self.suite_name} failed."
> > diff --git a/dts/framework/remote_session/__init__.py
> b/dts/framework/remote_session/__init__.py
> > index ee221503..4fe32d35 100644
> > --- a/dts/framework/remote_session/__init__.py
> > +++ b/dts/framework/remote_session/__init__.py
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: BSD-3-Clause
> >  # Copyright(c) 2023 PANTHEON.tech s.r.o.
> > +# Copyright(c) 2023 University of New Hampshire
> >
> >  """
> >  The package provides modules for managing remote connections to a
> remote host (node),
> > @@ -17,7 +18,14 @@
> >
> >  from .linux_session import LinuxSession
> >  from .os_session import OSSession
> > -from .remote import CommandResult, RemoteSession, SSHSession
> > +from .remote import (
> > +    CommandResult,
> > +    InteractiveRemoteSession,
> > +    InteractiveShell,
> > +    RemoteSession,
> > +    SSHSession,
> > +    TestPmdShell,
> > +)
> >
> >
> >  def create_session(
> > diff --git a/dts/framework/remote_session/os_session.py
> b/dts/framework/remote_session/os_session.py
> > index 4c48ae25..4346ecc4 100644
> > --- a/dts/framework/remote_session/os_session.py
> > +++ b/dts/framework/remote_session/os_session.py
> > @@ -6,13 +6,19 @@
> >  from collections.abc import Iterable
> >  from pathlib import PurePath
> >
> > -from framework.config import Architecture, NodeConfiguration
> > +from framework.config import Architecture, NodeConfiguration, NodeInfo
> >  from framework.logger import DTSLOG
> >  from framework.settings import SETTINGS
> >  from framework.testbed_model import LogicalCore
> >  from framework.utils import EnvVarsDict, MesonArgs
> >
> > -from .remote import CommandResult, RemoteSession, create_remote_session
> > +from .remote import (
> > +    CommandResult,
> > +    InteractiveRemoteSession,
> > +    RemoteSession,
> > +    create_interactive_session,
> > +    create_remote_session,
> > +)
> >
> >
> >  class OSSession(ABC):
> > @@ -26,6 +32,7 @@ class OSSession(ABC):
> >      name: str
> >      _logger: DTSLOG
> >      remote_session: RemoteSession
> > +    interactive_session: InteractiveRemoteSession
> >
> >      def __init__(
> >          self,
> > @@ -37,6 +44,7 @@ def __init__(
> >          self.name = name
> >          self._logger = logger
> >          self.remote_session = create_remote_session(node_config, name,
> logger)
> > +        self.interactive_session =
> create_interactive_session(node_config, name, logger)
> >
> >      def close(self, force: bool = False) -> None:
> >          """
> > @@ -173,3 +181,15 @@ def setup_hugepages(self, hugepage_amount: int,
> force_first_numa: bool) -> None:
> >          if needed and mount the hugepages if needed.
> >          If force_first_numa is True, configure hugepages just on the
> first socket.
> >          """
> > +
> > +    @abstractmethod
> > +    def get_compiler_version(self, compiler_name: str) -> str:
> > +        """
> > +        Get installed version of compiler used for DPDK
> > +        """
> > +
> > +    @abstractmethod
> > +    def get_node_info(self) -> NodeInfo:
> > +        """
> > +        Collect information about the node
> > +        """
> > diff --git a/dts/framework/remote_session/posix_session.py
> b/dts/framework/remote_session/posix_session.py
> > index d38062e8..f8ec159f 100644
> > --- a/dts/framework/remote_session/posix_session.py
> > +++ b/dts/framework/remote_session/posix_session.py
> > @@ -6,7 +6,7 @@
> >  from collections.abc import Iterable
> >  from pathlib import PurePath, PurePosixPath
> >
> > -from framework.config import Architecture
> > +from framework.config import Architecture, NodeInfo
> >  from framework.exception import DPDKBuildError,
> RemoteCommandExecutionError
> >  from framework.settings import SETTINGS
> >  from framework.utils import EnvVarsDict, MesonArgs
> > @@ -219,3 +219,30 @@ def _remove_dpdk_runtime_dirs(
> >
> >      def get_dpdk_file_prefix(self, dpdk_prefix) -> str:
> >          return ""
> > +
> > +    def get_compiler_version(self, compiler_name: str) -> str:
> > +        match compiler_name:
> > +            case "gcc":
> > +                return self.send_command(f"{compiler_name} --version",
> 60).stdout.split(
> > +                    "\n"
> > +                )[0]
>
> The timeouts are still there.
>
> > +            case "clang":
> > +                return self.send_command(f"{compiler_name} --version",
> 60).stdout.split(
> > +                    "\n"
> > +                )[0]
> > +            case "msvc":
> > +                return self.send_command("cl", 60).stdout
> > +            case "icc":
> > +                return self.send_command(f"{compiler_name} -V",
> 60).stdout
> > +            case _:
> > +                raise ValueError(f"Unknown compiler {compiler_name}")
> > +
> > +    def get_node_info(self) -> NodeInfo:
> > +        os_release_info = self.send_command(
> > +            "awk -F= '$1 ~ /^NAME$|^VERSION$/ {print $2}'
> /etc/os-release",
> > +            SETTINGS.timeout,
> > +        ).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
> > +        )
> > diff --git a/dts/framework/remote_session/remote/__init__.py
> b/dts/framework/remote_session/remote/__init__.py
> > index 8a151221..224598a8 100644
> > --- a/dts/framework/remote_session/remote/__init__.py
> > +++ b/dts/framework/remote_session/remote/__init__.py
> > @@ -1,16 +1,26 @@
> >  # SPDX-License-Identifier: BSD-3-Clause
> >  # Copyright(c) 2023 PANTHEON.tech s.r.o.
> > +# Copyright(c) 2022-2023 University of New Hampshire
>
> There are other instances of the copyright statement. It's not
> necessary to change them all to just 2023, but I'd say it's
> preferable.
>

I agree. Knowing more about them after your other comments, this should
also be changed.


>
> >
> >  # pylama:ignore=W0611
> >
> >  from framework.config import NodeConfiguration
> >  from framework.logger import DTSLOG
> >
> > +from .interactive_remote_session import InteractiveRemoteSession
> > +from .interactive_shell import InteractiveShell
> >  from .remote_session import CommandResult, RemoteSession
> >  from .ssh_session import SSHSession
> > +from .testpmd_shell import TestPmdShell
> >
> >
> >  def create_remote_session(
> >      node_config: NodeConfiguration, name: str, logger: DTSLOG
> >  ) -> RemoteSession:
> >      return SSHSession(node_config, name, logger)
> > +
> > +
> > +def create_interactive_session(
> > +    node_config: NodeConfiguration, name: str, logger: DTSLOG
> > +) -> InteractiveRemoteSession:
> > +    return InteractiveRemoteSession(node_config, logger)
> > diff --git
> a/dts/framework/remote_session/remote/interactive_remote_session.py
> b/dts/framework/remote_session/remote/interactive_remote_session.py
> > new file mode 100644
> > index 00000000..e145d35d
> > --- /dev/null
> > +++ b/dts/framework/remote_session/remote/interactive_remote_session.py
> > @@ -0,0 +1,118 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 University of New Hampshire
> > +
> > +import socket
> > +import traceback
> > +from pathlib import PurePath
> > +from typing import Union
> > +
> > +from paramiko import AutoAddPolicy, SSHClient, Transport  # type: ignore
> > +from paramiko.ssh_exception import (  # type: ignore
> > +    AuthenticationException,
> > +    BadHostKeyException,
> > +    NoValidConnectionsError,
> > +    SSHException,
> > +)
> > +
> > +from framework.config import InteractiveApp, NodeConfiguration
> > +from framework.exception import SSHConnectionError
> > +from framework.logger import DTSLOG
> > +
> > +from .interactive_shell import InteractiveShell
> > +from .testpmd_shell import TestPmdShell
> > +
> > +
> > +class InteractiveRemoteSession:
> > +    hostname: str
> > +    ip: str
> > +    port: int
> > +    username: str
> > +    password: str
> > +    _logger: DTSLOG
> > +    _node_config: NodeConfiguration
> > +    session: SSHClient
> > +    _transport: Transport | None
> > +
> > +    def __init__(self, node_config: NodeConfiguration, _logger: DTSLOG)
> -> None:
> > +        self._node_config = node_config
> > +        self._logger = _logger
> > +        self.hostname = node_config.hostname
> > +        self.username = node_config.user
> > +        self.password = node_config.password if node_config.password
> else ""
> > +        port = "22"
> > +        self.ip = node_config.hostname
> > +        if ":" in node_config.hostname:
> > +            self.ip, port = node_config.hostname.split(":")
> > +        self.port = int(port)
> > +        self._logger.info(
> > +            f"Initializing interactive connection for {self.username}@
> {self.hostname}"
> > +        )
> > +        self._connect()
> > +        self._logger.info(
> > +            f"Interactive connection successful for {self.username}@
> {self.hostname}"
> > +        )
> > +
> > +    def _connect(self) -> None:
> > +        client = SSHClient()
> > +        client.set_missing_host_key_policy(AutoAddPolicy)
> > +        self.session = client
> > +        retry_attempts = 10
> > +        for retry_attempt in range(retry_attempts):
> > +            try:
> > +                client.connect(
> > +                    self.ip,
> > +                    username=self.username,
> > +                    port=self.port,
> > +                    password=self.password,
> > +                    timeout=20 if self.port else 10,
> > +                )
> > +            except (TypeError, BadHostKeyException,
> AuthenticationException) as e:
> > +                self._logger.exception(e)
> > +                raise SSHConnectionError(self.hostname) from e
> > +            except (NoValidConnectionsError, socket.error,
> SSHException) as e:
> > +                self._logger.debug(traceback.format_exc())
> > +                self._logger.warning(e)
> > +                self._logger.info(
> > +                    "Retrying interactive session connection: "
> > +                    f"retry number {retry_attempt +1}"
> > +                )
> > +            else:
> > +                break
> > +        else:
> > +            raise SSHConnectionError(self.hostname)
> > +        # Interactive sessions are used on an "as needed" basis so we
> have
> > +        # to set a keepalive
> > +        self._transport = self.session.get_transport()
> > +        if self._transport is not None:
> > +            self._transport.set_keepalive(30)
> > +
> > +    def create_interactive_shell(
> > +        self,
> > +        shell_type: InteractiveApp,
> > +        path_to_app: PurePath,
> > +        eal_parameters: str,
> > +        timeout: float,
> > +    ) -> Union[InteractiveShell, TestPmdShell]:
> > +        """
> > +        See "create_interactive_shell" in SutNode
> > +        """
> > +        match (shell_type):
> > +            case InteractiveApp.shell:
> > +                return InteractiveShell(
> > +                    self.session, self._logger, path_to_app, timeout
> > +                )
> > +            case InteractiveApp.testpmd:
> > +                return TestPmdShell(
> > +                    self.session,
> > +                    self._logger,
> > +                    path_to_app,
> > +                    timeout=timeout,
> > +                    eal_flags=eal_parameters,
> > +                )
> > +            case _:
> > +                self._logger.info(
> > +                    f"Unhandled app type {shell_type.name}, defaulting
> to shell."
> > +                )
> > +                return InteractiveShell(
> > +                    self.session, self._logger, path_to_app, timeout
> > +                )
> > diff --git a/dts/framework/remote_session/remote/interactive_shell.py
> b/dts/framework/remote_session/remote/interactive_shell.py
> > new file mode 100644
> > index 00000000..4b0735c8
> > --- /dev/null
> > +++ b/dts/framework/remote_session/remote/interactive_shell.py
> > @@ -0,0 +1,99 @@
> > +from pathlib import PurePath
> > +
> > +from paramiko import Channel, SSHClient, channel  # type: ignore
> > +
> > +from framework.logger import DTSLOG
> > +from framework.settings import SETTINGS
> > +
> > +
> > +class InteractiveShell:
> > +
> > +    _interactive_session: SSHClient
> > +    _stdin: channel.ChannelStdinFile
> > +    _stdout: channel.ChannelFile
> > +    _ssh_channel: Channel
> > +    _logger: DTSLOG
> > +    _timeout: float
> > +    _path_to_app: PurePath
> > +
> > +    def __init__(
> > +        self,
> > +        interactive_session: SSHClient,
> > +        logger: DTSLOG,
> > +        path_to_app: PurePath,
> > +        timeout: float = SETTINGS.timeout,
> > +    ) -> None:
> > +        self._interactive_session = interactive_session
> > +        self._ssh_channel = self._interactive_session.invoke_shell()
> > +        self._stdin = self._ssh_channel.makefile_stdin("w")
> > +        self._stdout = self._ssh_channel.makefile("r")
> > +        self._ssh_channel.settimeout(timeout)
> > +        self._ssh_channel.set_combine_stderr(True)  # combines stdout
> and stderr streams
> > +        self._logger = logger
> > +        self._timeout = timeout
> > +        self._path_to_app = path_to_app
> > +
> > +
> > +    def send_command_no_output(self, command: str) -> None:
> > +        """Send command to channel without recording output.
> > +
> > +        This method will not verify any input or output, it will simply
> assume the
> > +        command succeeded. This method will also consume all output in
> the buffer
> > +        after executing the command.
> > +        """
> > +        self._logger.info(
> > +            f"Sending command {command.strip()} and not collecting
> output"
> > +        )
> > +        self._stdin.write(f"{command}\n")
> > +        self._stdin.flush()
> > +        self.empty_stdout_buffer()
> > +
> > +    def empty_stdout_buffer(self) -> None:
> > +        """Removes all data from the stdout buffer.
> > +
> > +        Because of the way paramiko handles read buffers, there is no
> way to effectively
> > +        remove data from, or "flush", read buffers. This method
> essentially moves our
> > +        offset on the buffer to the end and thus "removes" the data
> from the buffer.
> > +        Timeouts are thrown on read operations of paramiko pipes based
> on whether data
> > +        had been received before timeout so we assume that if we reach
> the timeout then
> > +        we are at the end of the buffer.
> > +        """
> > +        self._ssh_channel.settimeout(0.5)
> > +        try:
> > +            for line in self._stdout:
> > +                pass
> > +        except TimeoutError:
> > +            pass
> > +        self._ssh_channel.settimeout(self._timeout)  # reset timeout
> > +
> > +    def send_command_get_output(self, command: str, prompt: str) -> str:
> > +        """Send a command and get all output before the expected ending
> string.
> > +
> > +        Lines that expect input are not included in the stdout buffer
> so they cannot be
> > +        used for expect. For example, if you were prompted to log into
> something
> > +        with a username and password, you cannot expect "username:"
> because it won't
> > +        yet be in the stdout buffer. A work around for this could be
> consuming an
> > +        extra newline character to force the current prompt into the
> stdout buffer.
> > +
> > +        Returns:
> > +            All output in the buffer before expected string
> > +        """
> > +        self._logger.info(f"Sending command {command.strip()}...")
> > +        self._stdin.write(f"{command}\n")
> > +        self._stdin.flush()
> > +        out: str = ""
> > +        for line in self._stdout:
> > +            out += line
> > +            if prompt in line and not line.rstrip().endswith(
> > +                command.rstrip()
> > +            ):  # ignore line that sent command
> > +                break
> > +        self._logger.debug(f"Got output: {out}")
> > +        return out
> > +
> > +    def close(self) -> None:
> > +        self._stdin.close()
> > +        self._ssh_channel.close()
> > +
> > +    def __del__(self) -> None:
> > +        self.close()
> > diff --git a/dts/framework/remote_session/remote/testpmd_shell.py
> b/dts/framework/remote_session/remote/testpmd_shell.py
> > new file mode 100644
> > index 00000000..bde3468b
> > --- /dev/null
> > +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 University of New Hampshire
> > +
> > +
> > +from pathlib import PurePath
> > +
> > +from paramiko import SSHClient  # type: ignore
> > +
> > +from framework.logger import DTSLOG
> > +from framework.settings import SETTINGS
> > +
> > +from .interactive_shell import InteractiveShell
> > +
> > +
> > +class TestPmdShell(InteractiveShell):
> > +    expected_prompt: str = "testpmd>"
> > +    _eal_flags: str
> > +
> > +    def __init__(
> > +        self,
> > +        interactive_session: SSHClient,
> > +        logger: DTSLOG,
> > +        path_to_testpmd: PurePath,
> > +        eal_flags: str,
> > +        timeout: float = SETTINGS.timeout,
> > +    ) -> None:
> > +        """Initializes an interactive testpmd session using specified
> parameters."""
> > +        self._eal_flags = eal_flags
> > +
> > +        super(TestPmdShell, self).__init__(
> > +            interactive_session,
> > +            logger=logger,
> > +            path_to_app=path_to_testpmd,
> > +            timeout=timeout,
> > +        )
> > +        self._start_application()
> > +
> > +    def _start_application(self) -> None:
> > +        """Starts a new interactive testpmd shell using _path_to_app.
> > +        """
> > +        self.send_command(
> > +            f"{self._path_to_app} {self._eal_flags} -- -i",
> > +        )
> > +
> > +    def send_command(self, command: str, prompt: str = expected_prompt)
> -> str:
> > +        """Specific way of handling the command for testpmd
> > +
> > +        An extra newline character is consumed in order to force the
> current line into
> > +        the stdout buffer.
> > +        """
> > +        return self.send_command_get_output(f"{command}\n", prompt)
> > +
> > +    def get_devices(self) -> list[str]:
> > +        """Get a list of device names that are known to testpmd
> > +
> > +        Uses the device info listed in testpmd and then parses the
> output to
> > +        return only the names of the devices.
> > +
> > +        Returns:
> > +            A list of strings representing device names (e.g.
> 0000:14:00.1)
> > +        """
> > +        dev_info: str = self.send_command("show device info all")
> > +        dev_list: list[str] = []
> > +        for line in dev_info.split("\n"):
> > +            if "device name:" in line.lower():
> > +                dev_list.append(line.strip().split(": ")[1].strip())
> > +        return dev_list
> > diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py
> > index 74391982..2436eb7f 100644
> > --- a/dts/framework/test_result.py
> > +++ b/dts/framework/test_result.py
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: BSD-3-Clause
> >  # Copyright(c) 2023 PANTHEON.tech s.r.o.
> > +# Copyright(c) 2022-2023 University of New Hampshire
> >
> >  """
> >  Generic result container and reporters
> > @@ -13,9 +14,11 @@
> >      OS,
> >      Architecture,
> >      BuildTargetConfiguration,
> > +    BuildTargetInfo,
> >      Compiler,
> >      CPUType,
> >      NodeConfiguration,
> > +    NodeInfo,
> >  )
> >  from .exception import DTSError, ErrorSeverity
> >  from .logger import DTSLOG
> > @@ -67,12 +70,14 @@ class Statistics(dict):
> >      Using a dict provides a convenient way to format the data.
> >      """
> >
> > -    def __init__(self, dpdk_version):
> > +    def __init__(self, output_info: dict[str, str] | None):
> >          super(Statistics, self).__init__()
> >          for result in Result:
> >              self[result.name] = 0
> >          self["PASS RATE"] = 0.0
> > -        self["DPDK VERSION"] = dpdk_version
> > +        if output_info:
> > +            for info_key, info_val in output_info.items():
> > +                self[info_key] = info_val
> >
> >      def __iadd__(self, other: Result) -> "Statistics":
> >          """
> > @@ -206,6 +211,8 @@ class BuildTargetResult(BaseResult):
> >      os: OS
> >      cpu: CPUType
> >      compiler: Compiler
> > +    compiler_version: str | None
> > +    dpdk_version: str | None
> >
> >      def __init__(self, build_target: BuildTargetConfiguration):
> >          super(BuildTargetResult, self).__init__()
> > @@ -213,6 +220,12 @@ def __init__(self, build_target:
> BuildTargetConfiguration):
> >          self.os = build_target.os
> >          self.cpu = build_target.cpu
> >          self.compiler = build_target.compiler
> > +        self.compiler_version = None
> > +        self.dpdk_version = None
> > +
> > +    def add_build_target_versions(self, versions: BuildTargetInfo) ->
> None:
> > +        self.compiler_version = versions.compiler_version
> > +        self.dpdk_version = versions.dpdk_version
> >
> >      def add_test_suite(self, test_suite_name: str) -> TestSuiteResult:
> >          test_suite_result = TestSuiteResult(test_suite_name)
> > @@ -228,10 +241,17 @@ class ExecutionResult(BaseResult):
> >      """
> >
> >      sut_node: NodeConfiguration
> > +    sut_os_name: str
> > +    sut_os_version: str
> > +    sut_kernel_version: str
> >
> > -    def __init__(self, sut_node: NodeConfiguration):
> > +    def __init__(self, sut_node: NodeConfiguration, sut_version_info:
> NodeInfo):
> >          super(ExecutionResult, self).__init__()
> >          self.sut_node = sut_node
> > +        self.sut_version_info = sut_version_info
> > +        self.sut_os_name = sut_version_info.os_name
> > +        self.sut_os_version = sut_version_info.os_version
> > +        self.sut_kernel_version = sut_version_info.kernel_version
> >
> >      def add_build_target(
> >          self, build_target: BuildTargetConfiguration
> > @@ -258,6 +278,7 @@ class DTSResult(BaseResult):
> >      """
> >
> >      dpdk_version: str | None
> > +    output: dict | None
> >      _logger: DTSLOG
> >      _errors: list[Exception]
> >      _return_code: ErrorSeverity
> > @@ -267,14 +288,17 @@ class DTSResult(BaseResult):
> >      def __init__(self, logger: DTSLOG):
> >          super(DTSResult, self).__init__()
> >          self.dpdk_version = None
> > +        self.output = None
> >          self._logger = logger
> >          self._errors = []
> >          self._return_code = ErrorSeverity.NO_ERR
> >          self._stats_result = None
> >          self._stats_filename = os.path.join(SETTINGS.output_dir,
> "statistics.txt")
> >
> > -    def add_execution(self, sut_node: NodeConfiguration) ->
> ExecutionResult:
> > -        execution_result = ExecutionResult(sut_node)
> > +    def add_execution(
> > +        self, sut_node: NodeConfiguration, sut_version_info: NodeInfo
> > +    ) -> ExecutionResult:
> > +        execution_result = ExecutionResult(sut_node, sut_version_info)
> >          self._inner_results.append(execution_result)
> >          return execution_result
> >
> > @@ -296,7 +320,8 @@ def process(self) -> None:
> >              for error in self._errors:
> >                  self._logger.debug(repr(error))
> >
> > -        self._stats_result = Statistics(self.dpdk_version)
> > +        self._stats_result = Statistics(self.output)
> > +        # add information gathered from the smoke tests to the
> statistics
> >          self.add_stats(self._stats_result)
> >          with open(self._stats_filename, "w+") as stats_file:
> >              stats_file.write(str(self._stats_result))
> > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> > index 0705f38f..5df5d2a6 100644
> > --- a/dts/framework/test_suite.py
> > +++ b/dts/framework/test_suite.py
> > @@ -11,10 +11,21 @@
> >  import re
> >  from types import MethodType
> >
> > -from .exception import ConfigurationError, SSHTimeoutError,
> TestCaseVerifyError
> > +from .exception import (
> > +    BlockingTestSuiteError,
> > +    ConfigurationError,
> > +    SSHTimeoutError,
> > +    TestCaseVerifyError,
> > +)
> >  from .logger import DTSLOG, getLogger
> >  from .settings import SETTINGS
> > -from .test_result import BuildTargetResult, Result, TestCaseResult,
> TestSuiteResult
> > +from .test_result import (
> > +    BuildTargetResult,
> > +    DTSResult,
> > +    Result,
> > +    TestCaseResult,
> > +    TestSuiteResult,
> > +)
> >  from .testbed_model import SutNode
> >
> >
> > @@ -37,10 +48,12 @@ class TestSuite(object):
> >      """
> >
> >      sut_node: SutNode
> > +    is_blocking = False
> >      _logger: DTSLOG
> >      _test_cases_to_run: list[str]
> >      _func: bool
> >      _result: TestSuiteResult
> > +    _dts_result: DTSResult
> >
> >      def __init__(
> >          self,
> > @@ -48,6 +61,7 @@ def __init__(
> >          test_cases: list[str],
> >          func: bool,
> >          build_target_result: BuildTargetResult,
> > +        dts_result: DTSResult,
> >      ):
> >          self.sut_node = sut_node
> >          self._logger = getLogger(self.__class__.__name__)
> > @@ -55,6 +69,7 @@ def __init__(
> >          self._test_cases_to_run.extend(SETTINGS.test_cases)
> >          self._func = func
> >          self._result =
> build_target_result.add_test_suite(self.__class__.__name__)
> > +        self._dts_result = dts_result
> >
> >      def set_up_suite(self) -> None:
> >          """
> > @@ -118,6 +133,8 @@ def run(self) -> None:
> >                      f"the next test suite may be affected."
> >                  )
> >                  self._result.update_setup(Result.ERROR, e)
> > +            if len(self._result.get_errors()) > 0 and self.is_blocking:
> > +                raise BlockingTestSuiteError(test_suite_name)
> >
> >      def _execute_test_suite(self) -> None:
> >          """
> > diff --git a/dts/framework/testbed_model/node.py
> b/dts/framework/testbed_model/node.py
> > index d48fafe6..c5147e0e 100644
> > --- a/dts/framework/testbed_model/node.py
> > +++ b/dts/framework/testbed_model/node.py
> > @@ -40,6 +40,7 @@ class Node(object):
> >      lcores: list[LogicalCore]
> >      _logger: DTSLOG
> >      _other_sessions: list[OSSession]
> > +    _execution_config: ExecutionConfiguration
> >
> >      def __init__(self, node_config: NodeConfiguration):
> >          self.config = node_config
> > @@ -64,6 +65,7 @@ def set_up_execution(self, execution_config:
> ExecutionConfiguration) -> None:
> >          """
> >          self._setup_hugepages()
> >          self._set_up_execution(execution_config)
> > +        self._execution_config = execution_config
> >
> >      def _set_up_execution(self, execution_config:
> ExecutionConfiguration) -> None:
> >          """
> > diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> > index 2b2b50d9..9b17ac3d 100644
> > --- a/dts/framework/testbed_model/sut_node.py
> > +++ b/dts/framework/testbed_model/sut_node.py
> > @@ -1,14 +1,27 @@
> >  # SPDX-License-Identifier: BSD-3-Clause
> >  # Copyright(c) 2010-2014 Intel Corporation
> >  # Copyright(c) 2023 PANTHEON.tech s.r.o.
> > +# Copyright(c) 2022-2023 University of New Hampshire
> >
> >  import os
> >  import tarfile
> >  import time
> >  from pathlib import PurePath
> > -
> > -from framework.config import BuildTargetConfiguration, NodeConfiguration
> > -from framework.remote_session import CommandResult, OSSession
> > +from typing import Union
> > +
> > +from framework.config import (
> > +    BuildTargetConfiguration,
> > +    BuildTargetInfo,
> > +    InteractiveApp,
> > +    NodeConfiguration,
> > +    NodeInfo,
> > +)
> > +from framework.remote_session import (
> > +    CommandResult,
> > +    InteractiveShell,
> > +    OSSession,
> > +    TestPmdShell,
> > +)
> >  from framework.settings import SETTINGS
> >  from framework.utils import EnvVarsDict, MesonArgs
> >
> > @@ -16,6 +29,52 @@
> >  from .node import Node
> >
> >
> > +class EalParameters(object):
> > +    def __init__(
> > +        self,
> > +        lcore_list: LogicalCoreList,
> > +        memory_channels: int,
> > +        prefix: str,
> > +        no_pci: bool,
> > +        vdevs: list[VirtualDevice],
> > +        other_eal_param: str,
> > +    ):
> > +        """
> > +        Generate eal parameters character string;
> > +        :param lcore_list: the list of logical cores to use.
> > +        :param memory_channels: the number of memory channels to use.
> > +        :param prefix: set file prefix string, eg:
> > +                        prefix='vf'
> > +        :param no_pci: switch of disable PCI bus eg:
> > +                        no_pci=True
> > +        :param vdevs: virtual device list, eg:
> > +                        vdevs=[
> > +                            VirtualDevice('net_ring0'),
> > +                            VirtualDevice('net_ring1')
> > +                        ]
> > +        :param other_eal_param: user defined DPDK eal parameters, eg:
> > +                        other_eal_param='--single-file-segments'
> > +        """
> > +        self._lcore_list = f"-l {lcore_list}"
> > +        self._memory_channels = f"-n {memory_channels}"
> > +        self._prefix = prefix
> > +        if prefix:
> > +            self._prefix = f"--file-prefix={prefix}"
> > +        self._no_pci = "--no-pci" if no_pci else ""
> > +        self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
> > +        self._other_eal_param = other_eal_param
> > +
> > +    def __str__(self) -> str:
> > +        return (
> > +            f"{self._lcore_list} "
> > +            f"{self._memory_channels} "
> > +            f"{self._prefix} "
> > +            f"{self._no_pci} "
> > +            f"{self._vdevs} "
> > +            f"{self._other_eal_param}"
> > +        )
> > +
> > +
> >  class SutNode(Node):
> >      """
> >      A class for managing connections to the System under Test, providing
> > @@ -30,9 +89,11 @@ class SutNode(Node):
> >      _env_vars: EnvVarsDict
> >      _remote_tmp_dir: PurePath
> >      __remote_dpdk_dir: PurePath | None
> > -    _dpdk_version: str | None
> >      _app_compile_timeout: float
> >      _dpdk_kill_session: OSSession | None
> > +    _dpdk_version: str | None
> > +    _node_info: NodeInfo | None
> > +    _compiler_version: str | None
> >
> >      def __init__(self, node_config: NodeConfiguration):
> >          super(SutNode, self).__init__(node_config)
> > @@ -41,12 +102,14 @@ def __init__(self, node_config: NodeConfiguration):
> >          self._env_vars = EnvVarsDict()
> >          self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
> >          self.__remote_dpdk_dir = None
> > -        self._dpdk_version = None
> >          self._app_compile_timeout = 90
> >          self._dpdk_kill_session = None
> >          self._dpdk_timestamp = (
> >              f"{str(os.getpid())}_{time.strftime('%Y%m%d%H%M%S',
> time.localtime())}"
> >          )
> > +        self._dpdk_version = None
> > +        self._node_info = None
> > +        self._compiler_version = None
> >
> >      @property
> >      def _remote_dpdk_dir(self) -> PurePath:
> > @@ -75,6 +138,32 @@ def dpdk_version(self) -> str:
> >              )
> >          return self._dpdk_version
> >
> > +    @property
> > +    def node_info(self) -> NodeInfo:
> > +        if self._node_info is None:
> > +            self._node_info = self.main_session.get_node_info()
> > +        return self._node_info
> > +
> > +    @property
> > +    def compiler_version(self) -> str:
> > +        if self._compiler_version is None:
> > +            if self._build_target_config is not None:
> > +                self._compiler_version =
> self.main_session.get_compiler_version(
> > +                    self._build_target_config.compiler.name
> > +                )
> > +            else:
> > +                self._logger.warning(
> > +                    "Failed to get compiler version because"
> > +                    "_build_target_config is None."
> > +                )
> > +                return ""
> > +        return self._compiler_version
> > +
> > +    def get_build_target_info(self) -> BuildTargetInfo:
> > +        return BuildTargetInfo(
> > +            dpdk_version=self.dpdk_version,
> compiler_version=self.compiler_version
> > +        )
> > +
> >      def _guess_dpdk_remote_dir(self) -> PurePath:
> >          return
> self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
> >
> > @@ -84,6 +173,10 @@ def _set_up_build_target(
> >          """
> >          Setup DPDK on the SUT node.
> >          """
> > +        # we want to ensure that dpdk_version and compiler_version is
> reset for new
> > +        # build targets
> > +        self._dpdk_version = None
> > +        self._compiler_version = None
> >          self._configure_build_target(build_target_config)
> >          self._copy_dpdk_tarball()
> >          self._build_dpdk()
> > @@ -262,48 +355,43 @@ def run_dpdk_app(
> >              f"{app_path} {eal_args}", timeout, verify=True
> >          )
> >
> > -
> > -class EalParameters(object):
> > -    def __init__(
> > +    def create_interactive_shell(
> >          self,
> > -        lcore_list: LogicalCoreList,
> > -        memory_channels: int,
> > -        prefix: str,
> > -        no_pci: bool,
> > -        vdevs: list[VirtualDevice],
> > -        other_eal_param: str,
> > -    ):
> > +        shell_type: InteractiveApp,
> > +        path_to_app: PurePath | None = None,
> > +        timeout: float = SETTINGS.timeout,
> > +        eal_parameters: EalParameters | None = None,
> > +    ) -> Union[InteractiveShell, TestPmdShell]:
> > +        """Create a handler for an interactive session.
> > +
> > +        This method is a factory that calls a method in OSSession to
> create shells for
> > +        different DPDK applications.
> > +
> > +        Args:
> > +            shell_type: Enum value representing the desired application.
> > +            path_to_app: Represents a path to the application you are
> attempting to
> > +                launch. This path will be executed at the start of the
> app
> > +                initialization. If one isn't provided, the default
> specified in the
> > +                enumeration will be used.
> > +            timeout: Timeout for reading output from the SSH channel.
> If you are
> > +                reading from the buffer and don't receive any data
> within the timeout
> > +                it will throw an error.
> > +            eal_parameters: List of EAL parameters to use to launch the
> app. This is
> > +                ignored for base "shell" types.
> > +        Returns:
> > +            Instance of the desired interactive application.
> >          """
> > -        Generate eal parameters character string;
> > -        :param lcore_list: the list of logical cores to use.
> > -        :param memory_channels: the number of memory channels to use.
> > -        :param prefix: set file prefix string, eg:
> > -                        prefix='vf'
> > -        :param no_pci: switch of disable PCI bus eg:
> > -                        no_pci=True
> > -        :param vdevs: virtual device list, eg:
> > -                        vdevs=[
> > -                            VirtualDevice('net_ring0'),
> > -                            VirtualDevice('net_ring1')
> > -                        ]
> > -        :param other_eal_param: user defined DPDK eal parameters, eg:
> > -                        other_eal_param='--single-file-segments'
> > -        """
> > -        self._lcore_list = f"-l {lcore_list}"
> > -        self._memory_channels = f"-n {memory_channels}"
> > -        self._prefix = prefix
> > -        if prefix:
> > -            self._prefix = f"--file-prefix={prefix}"
> > -        self._no_pci = "--no-pci" if no_pci else ""
> > -        self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
> > -        self._other_eal_param = other_eal_param
> > +        # if we just want a default shell, there is no need to append
> the DPDK build
> > +        # directory to the path
> > +        default_path = shell_type.get_path()
> >
> > -    def __str__(self) -> str:
> > -        return (
> > -            f"{self._lcore_list} "
> > -            f"{self._memory_channels} "
> > -            f"{self._prefix} "
> > -            f"{self._no_pci} "
> > -            f"{self._vdevs} "
> > -            f"{self._other_eal_param}"
> > +        if shell_type != InteractiveApp.shell:
> > +            default_path = self.main_session.join_remote_path(
> > +                self.remote_dpdk_build_dir, shell_type.get_path()
> > +            )
> > +        return
> self.main_session.interactive_session.create_interactive_shell(
> > +            shell_type,
> > +            path_to_app if path_to_app else default_path,
> > +            str(eal_parameters) if eal_parameters else "",
> > +            timeout,
> >          )
>
> I forgot to mention that I'd like to change to structure a bit here -
> calling self.main_session.create_interactive_shell() would makes more
> sense to me, as the interactive_session is basically an implementation
> detail.
>

Sure, I'll make that change as well.


>
> > diff --git a/dts/tests/TestSuite_smoke_tests.py
> b/dts/tests/TestSuite_smoke_tests.py
> > new file mode 100644
> > index 00000000..b7e70ee1
> > --- /dev/null
> > +++ b/dts/tests/TestSuite_smoke_tests.py
> > @@ -0,0 +1,118 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 University of New Hampshire
> > +
> > +import re
> > +
> > +from framework.config import InteractiveApp
> > +from framework.remote_session import TestPmdShell
> > +from framework.settings import SETTINGS
> > +from framework.test_suite import TestSuite
> > +
> > +
> > +class SmokeTests(TestSuite):
> > +    is_blocking = True
> > +    # dicts in this list are expected to have two keys:
> > +    # "pci_address" and "current_driver"
> > +    nics_in_node: list[dict[str, str]] = []
> > +
> > +    def set_up_suite(self) -> None:
> > +        """
> > +        Setup:
> > +            Set the build directory path and generate a list of NICs in
> the SUT node.
> > +        """
> > +        self.dpdk_build_dir_path = self.sut_node.remote_dpdk_build_dir
> > +        for nic in self.sut_node.config.ports:
> > +            new_dict = {
> > +                "pci_address": nic.pci,
> > +                "current_driver": nic.os_driver.strip(),
> > +            }
> > +            self.nics_in_node.append(new_dict)
> > +
> > +    def test_unit_tests(self) -> None:
> > +        """
> > +        Test:
> > +            Run the fast-test unit-test suite through meson.
> > +        """
> > +        self.sut_node.main_session.send_command(
> > +            f"meson test -C {self.dpdk_build_dir_path} --suite
> fast-tests",
> > +            300,
> > +            verify=True,
> > +        )
> > +
> > +    def test_driver_tests(self) -> None:
> > +        """
> > +        Test:
> > +            Run the driver-test unit-test suite through meson.
> > +        """
> > +        list_of_vdevs = ""
> > +        for dev in self.sut_node._execution_config.vdevs:
> > +            list_of_vdevs += f"--vdev {dev} "
> > +        list_of_vdevs = list_of_vdevs[:-1]
> > +        if list_of_vdevs:
> > +            self._logger.info(
> > +                "Running driver tests with the following virtual "
> > +                f"devices: {list_of_vdevs}"
> > +            )
> > +            self.sut_node.main_session.send_command(
> > +                f"meson test -C {self.dpdk_build_dir_path} --suite
> driver-tests "
> > +                f'--test-args "{list_of_vdevs}"',
> > +                300,
> > +                verify=True,
> > +            )
> > +        else:
> > +            self.sut_node.main_session.send_command(
> > +                f"meson test -C {self.dpdk_build_dir_path} --suite
> driver-tests",
> > +                300,
> > +                verify=True,
> > +            )
> > +
> > +    def test_devices_listed_in_testpmd(self) -> None:
> > +        """
> > +        Test:
> > +            Uses testpmd driver to verify that devices have been found
> by testpmd.
> > +        """
> > +        testpmd_driver =
> self.sut_node.create_interactive_shell(InteractiveApp.testpmd)
> > +        # We know it should always be a TestPmdShell but mypy doesn't
> > +        assert isinstance(testpmd_driver, TestPmdShell)
> > +        dev_list: list[str] = testpmd_driver.get_devices()
> > +        for nic in self.nics_in_node:
> > +            self.verify(
> > +                nic["pci_address"] in dev_list,
> > +                f"Device {nic['pci_address']} was not listed in
> testpmd's available devices, "
> > +                "please check your configuration",
> > +            )
> > +
> > +    def test_device_bound_to_driver(self) -> None:
> > +        """
> > +        Test:
> > +            Ensure that all drivers listed in the config are bound to
> the correct driver.
> > +        """
> > +        path_to_devbind = self.sut_node.main_session.join_remote_path(
> > +            self.sut_node._remote_dpdk_dir, "usertools",
> "dpdk-devbind.py"
> > +        )
> > +
> > +        regex_for_pci_address = "/[0-9]{4}:[0-9]{2}:[0-9]{2}.[0-9]{1}/"
>
> This shouldn't be tucked away in a test case. It should be in some
> high-level module - let's put it to utils.py for the time being.
>

Good point, this could be something that is useful in many other places as
well. I just created it there because that's just where I needed it but it
would be good to allow other people to reach it easily.


>
> > +        all_nics_in_dpdk_devbind =
> self.sut_node.main_session.send_command(
> > +            f"{path_to_devbind} --status | awk
> '{regex_for_pci_address}'",
> > +            SETTINGS.timeout,
> > +        ).stdout
> > +
> > +        for nic in self.nics_in_node:
> > +            # This regular expression finds the line in the above
> string that starts
> > +            # with the address for the nic we are on in the loop and
> then captures the
> > +            # name of the driver in a group
> > +            devbind_info_for_nic = re.search(
> > +                f"{nic['pci_address']}[^\\n]*drv=([\\d\\w]*) [^\\n]*",
> > +                all_nics_in_dpdk_devbind,
> > +            )
> > +            self.verify(
> > +                devbind_info_for_nic is not None,
> > +                f"Failed to find configured device
> ({nic['pci_address']}) using dpdk-devbind.py",
> > +            )
> > +            # We know this isn't None, but mypy doesn't
> > +            assert devbind_info_for_nic is not None
> > +            self.verify(
> > +                devbind_info_for_nic.group(1) == nic["current_driver"],
> > +                f"Driver for device {nic['pci_address']} does not match
> driver listed in "
> > +                f"configuration (bound to
> {devbind_info_for_nic.group(1)})",
> > +            )
> > --
> > 2.41.0
> >
>

Thank you for the comments!
Jeremy

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