DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jeremy Spewock <jspewock@iol.unh.edu>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net,
	lijuan.tu@intel.com,  wathsala.vithanage@arm.com,
	probb@iol.unh.edu, dev@dpdk.org
Subject: Re: [PATCH v8 1/1] dts: add smoke tests
Date: Tue, 18 Jul 2023 11:18:55 -0400	[thread overview]
Message-ID: <CAAA20URPFZ7c3w53Rqop9qWe12HbApXMAFqZ7mFJAG_0yUSc0Q@mail.gmail.com> (raw)
In-Reply-To: <CAOb5WZYB2p4m9kVh0zm1u91jcLSoOR4bh3ynZSEddVarppofRg@mail.gmail.com>

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

On Tue, Jul 18, 2023 at 8:08 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> A few more things I noticed while running DTS.
>
> On Mon, Jul 17, 2023 at 9:37 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              |  79 ++++++--
> >  dts/framework/config/conf_yaml_schema.json    | 142 ++++++++++++++-
> >  dts/framework/dts.py                          |  84 ++++++---
> >  dts/framework/exception.py                    |  12 ++
> >  dts/framework/remote_session/__init__.py      |  13 +-
> >  dts/framework/remote_session/os_session.py    |  48 ++++-
> >  dts/framework/remote_session/posix_session.py |  29 ++-
> >  .../remote_session/remote/__init__.py         |  10 ++
> >  .../remote/interactive_remote_session.py      |  82 +++++++++
> >  .../remote/interactive_shell.py               |  98 ++++++++++
> >  .../remote_session/remote/testpmd_shell.py    |  46 +++++
> >  dts/framework/test_result.py                  |  24 ++-
> >  dts/framework/test_suite.py                   |  10 +-
> >  dts/framework/testbed_model/node.py           |  43 ++++-
> >  dts/framework/testbed_model/sut_node.py       | 169 +++++++++++++-----
> >  dts/framework/utils.py                        |   3 +
> >  dts/tests/TestSuite_smoke_tests.py            | 114 ++++++++++++
> >  18 files changed, 931 insertions(+), 92 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 129801d87c..3a5d87cb49 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 skip
> smoke tests
>
> Typo: allows
>
>
Good catch, thank you.


> >
> >      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
> > +        - "crypto_openssl"
> >  nodes:
> >    - name: "SUT 1"
> >      hostname: sut1.change.me.localhost
>
> <snip>
>
> > diff --git a/dts/framework/config/conf_yaml_schema.json
> b/dts/framework/config/conf_yaml_schema.json
> > index ca2d4a1ef2..09fcbaf498 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.",
>
> Typo: extra an in are an inconsistencies
>
>
Fixed, thank you.



> > +              "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,
>
> <snip>
>
> > 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 0000000000..4d9c7638a5
> > --- /dev/null
> > +++ b/dts/framework/remote_session/remote/interactive_shell.py
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 University of New Hampshire
> > +
> > +from pathlib import PurePath
> > +from typing import Callable
> > +
> > +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
> > +    _startup_command: str
> > +    _app_args: str
> > +    _default_prompt: str = ""
> > +    _privileged: bool
> > +    _get_privileged_command: Callable[[str], str]
> > +    # Allows for app specific extra characters to be appended to
> commands
> > +    _command_extra_chars: str = ""
> > +    path: PurePath
> > +    dpdk_app: bool = False
> > +
> > +    def __init__(
> > +        self,
> > +        interactive_session: SSHClient,
> > +        logger: DTSLOG,
> > +        startup_command: str,
> > +        privileged: bool,
> > +        _get_privileged_command: Callable[[str], str],
> > +        app_args: str = "",
> > +        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._startup_command = startup_command
> > +        self._app_args = app_args
> > +        self._get_privileged_command = _get_privileged_command  # type:
> ignore
> > +        self._privileged = privileged
> > +        self._start_application()
> > +
> > +    def _start_application(self) -> None:
> > +        """Starts a new interactive application based on
> _startup_command.
> > +
> > +        This method is often overridden by subclasses as their process
> for
> > +        starting may look different.
> > +        """
> > +        start_command = f"{self._startup_command} {self._app_args}"
> > +        if self._privileged:
> > +            start_command =
> self._get_privileged_command(start_command)  # type: ignore
> > +        self.send_command(start_command)
> > +
> > +    def send_command(self, command: str, prompt: str | None = None) ->
> 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()}...")
>
> Let's unite this log with with remote remote session:
> self._logger.info(
> f"Sending: '{command}'" + (f" with env vars: '{env}'" if env else "")
> )
>
> We don't have env vars, but the rest should be the same.
>
>
That makes sense, we want to be sure that it's uniform. I'll fix this.


>
> > +        if prompt is None:
> > +            prompt = self._default_prompt
> > +        self._stdin.write(f"{command}{self._command_extra_chars}\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()
>

Thank you again for the comments,
Jeremy

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

      reply	other threads:[~2023-07-18 15:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-17 19:33 [PATCH v8 0/1] Add DTS " jspewock
2023-07-17 19:33 ` [PATCH v8 1/1] dts: add " jspewock
2023-07-18  9:41   ` Juraj Linkeš
2023-07-18 15:17     ` Jeremy Spewock
2023-07-18 12:08   ` Juraj Linkeš
2023-07-18 15:18     ` Jeremy Spewock [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAAA20URPFZ7c3w53Rqop9qWe12HbApXMAFqZ7mFJAG_0yUSc0Q@mail.gmail.com \
    --to=jspewock@iol.unh.edu \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=lijuan.tu@intel.com \
    --cc=probb@iol.unh.edu \
    --cc=thomas@monjalon.net \
    --cc=wathsala.vithanage@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).