From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: jspewock@iol.unh.edu
Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net,
wathsala.vithanage@arm.com, probb@iol.unh.edu,
paul.szczepanek@arm.com, yoan.picchi@foss.arm.com,
lylavoie@iol.unh.edu, ferruh.yigit@amd.com,
andrew.rybchenko@oktetlabs.ru, dev@dpdk.org
Subject: Re: [PATCH v6 1/7] dts: add startup verification and forwarding modes to testpmd shell
Date: Mon, 8 Jan 2024 12:34:57 +0100 [thread overview]
Message-ID: <CAOb5WZacHRBpBfgLOBY0Br2GYJa9pOu9BMw4TK-g=Fw3pN+pHg@mail.gmail.com> (raw)
In-Reply-To: <20240103223206.23129-2-jspewock@iol.unh.edu>
On Wed, Jan 3, 2024 at 11:33 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Added commonly used methods in testpmd such as starting and stopping
> packet forwarding, changing forward modes, and verifying link status of
> ports so that developers can configure testpmd and start forwarding
> through the provided class rather than sending commands to the testpmd
> session directly.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/exception.py | 7 +
> dts/framework/remote_session/testpmd_shell.py | 149 +++++++++++++++++-
> 2 files changed, 155 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/exception.py b/dts/framework/exception.py
> index 658eee2c38..cce1e0231a 100644
> --- a/dts/framework/exception.py
> +++ b/dts/framework/exception.py
<snip>
> @@ -65,9 +108,66 @@ class TestPmdShell(InteractiveShell):
> _command_extra_chars: ClassVar[str] = "\n"
>
> def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
> - self._app_args += " -- -i"
> + """Overrides :meth:`~.interactive_shell._start_application`.
> +
> + Add flags for starting testpmd in interactive mode and disabling messages for link state
> + change events before starting the application. Link state is verified before starting
> + packet forwarding and the messages create unexpected newlines in the terminal which
> + complicates output collection.
> +
We should adjust the collection so that it can handle the newlines.
Also, can you explain exactly why we are disabling the initial link
state messages?
> + Also find the number of pci addresses which were allowed on the command line when the app
> + was started.
> + """
> + self._app_args += " -- -i --mask-event intr_lsc"
> + self.number_of_ports = self._app_args.count("-a ")
> super()._start_application(get_privileged_command)
>
> + def start(self, verify: bool = True) -> None:
> + """Start packet forwarding with the current configuration.
> +
> + Args:
> + verify: If :data:`True` , a second start command will be sent in an attempt to verify
> + packet forwarding started as expected.
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True` and forwarding fails to
> + start or ports fail to come up.
> + """
> + self.send_command("start")
> + if verify:
> + # If forwarding was already started, sending "start" again should tell us
> + start_cmd_output = self.send_command("start")
> + if "Packet forwarding already started" not in start_cmd_output:
> + self._logger.debug(f"Failed to start packet forwarding: \n{start_cmd_output}")
> + raise InteractiveCommandExecutionError("Testpmd failed to start packet forwarding.")
> +
> + for port_id in range(self.number_of_ports):
> + if not self.wait_link_status_up(port_id):
> + raise InteractiveCommandExecutionError(
> + "Not all ports came up after starting packet forwarding in testpmd."
> + )
> +
> + def stop(self, verify: bool = True) -> None:
> + """Stop packet forwarding.
> +
> + Args:
> + verify: If :data:`True` , the output of the stop command is scanned to verify that
> + forwarding was stopped successfully or not started. If neither is found, it is
> + considered an error.
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True` and the command to stop
> + forwarding results in an error.
> + """
> + stop_cmd_output = self.send_command("stop")
> + if verify:
> + if (
> + "Done." not in stop_cmd_output
> + and "Packet forwarding not started" not in stop_cmd_output
> + ):
I want to make sure I understand this condition. When none of these
appear, it's an error. When just "Done." appears, we successfully
stopped ongoing forwarding and when "Packet forwarding not started"
appears, we're trying to stop forwarding that didn't start (or isn't
ongoing - it could've stopped in the meantime)?
I'm thinking about false failures here (Is there a string that would
indicate a failure even if one of the strings is printed?) - we're
basically looking at "not success" instead of looking for strings
telling us about a failure explicitly. Does the stop command not
produce such output? Or do we not know all of the failure strings or
is looking for the above two strings sufficient to rule out false
failures?
> + self._logger.debug(f"Failed to stop packet forwarding: \n{stop_cmd_output}")
> + raise InteractiveCommandExecutionError("Testpmd failed to stop packet forwarding.")
> +
> def get_devices(self) -> list[TestPmdDevice]:
> """Get a list of device names that are known to testpmd.
>
> @@ -82,3 +182,50 @@ def get_devices(self) -> list[TestPmdDevice]:
> if "device name:" in line.lower():
> dev_list.append(TestPmdDevice(line))
> return dev_list
> +
> + def wait_link_status_up(self, port_id: int, timeout=SETTINGS.timeout) -> bool:
> + """Wait until the link status on the given port is "up".
> +
> + Arguments:
> + port_id: Port to check the link status on.
> + timeout: Time to wait for the link to come up. The default value for this
> + argument is set using the :option:`-t, --timeout` command-line argument
> + or the :envvar:`DTS_TIMEOUT` environment variable.
> +
This really should be "may be modified", as it is optional.
> + Returns:
> + Whether the link came up in time or not.
> + """
> + time_to_stop = time.time() + timeout
> + port_info: str = ""
> + while time.time() < time_to_stop:
> + port_info = self.send_command(f"show port info {port_id}")
> + if "Link status: up" in port_info:
> + break
> + time.sleep(0.5)
> + else:
> + self._logger.error(f"The link for port {port_id} did not come up in the given timeout.")
> + return "Link status: up" in port_info
> +
> + def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True):
> + """Set packet forwarding mode.
> +
> + Args:
> + mode: The forwarding mode to use.
> + verify: If :data:`True` the output of the command will be scanned in an attempt to
> + verify that the forwarding mode was set to `mode` properly.
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True` and forwarding mode fails
I think there should be a definite article here - the forwarding mode.
> + to update.
> + """
> + set_fwd_output = self.send_command(f"set fwd {mode.value}")
> + if f"Set {mode.value} packet forwarding mode" not in set_fwd_output:
> + self._logger.debug(f"Failed to set fwd mode to {mode.value}:\n{set_fwd_output}")
> + raise InteractiveCommandExecutionError(
> + f"Test pmd failed to set fwd mode to {mode.value}"
> + )
> +
> + def close(self) -> None:
> + """Overrides :meth:`~.interactive_shell.close`."""
> + self.send_command("quit", "")
> + return super().close()
> --
> 2.43.0
>
next prev parent reply other threads:[~2024-01-08 11:35 UTC|newest]
Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-18 18:12 [PATCH v4 0/7] dts: Port scatter suite over jspewock
2023-12-18 18:12 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
2023-12-19 16:45 ` Juraj Linkeš
2023-12-21 19:37 ` Jeremy Spewock
2024-01-03 11:10 ` Juraj Linkeš
2023-12-18 18:12 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
2023-12-19 16:50 ` Juraj Linkeš
2023-12-18 18:12 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2023-12-19 16:54 ` Juraj Linkeš
2023-12-18 18:12 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
2023-12-19 16:55 ` Juraj Linkeš
2023-12-18 18:12 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
2023-12-19 16:58 ` Juraj Linkeš
2023-12-18 18:12 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
2023-12-19 16:59 ` Juraj Linkeš
2023-12-18 18:12 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
2023-12-19 17:29 ` Juraj Linkeš
2023-12-21 21:47 ` Jeremy Spewock
2024-01-03 11:14 ` Juraj Linkeš
2024-01-03 22:12 ` [PATCH v5 0/7] dts: Port scatter suite over jspewock
2024-01-03 22:12 ` [PATCH v5 1/7] dts: add startup verification and forwarding modes to testpmd shell jspewock
2024-01-03 22:12 ` [PATCH v5 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps jspewock
2024-01-03 22:12 ` [PATCH v5 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2024-01-03 22:12 ` [PATCH v5 4/7] dts: add pci addresses to EAL parameters jspewock
2024-01-03 22:12 ` [PATCH v5 5/7] dts: allow configuring MTU of ports jspewock
2024-01-03 22:12 ` [PATCH v5 6/7] dts: add scatter to the yaml schema jspewock
2024-01-03 22:12 ` [PATCH v5 7/7] dts: add pmd_buffer_scatter test suite jspewock
2024-01-03 22:31 ` [PATCH v6 0/7] dts: Port scatter suite over jspewock
2024-01-03 22:32 ` [PATCH v6 1/7] dts: add startup verification and forwarding modes to testpmd shell jspewock
2024-01-08 11:34 ` Juraj Linkeš [this message]
2024-01-08 16:36 ` Jeremy Spewock
2024-01-09 11:54 ` Juraj Linkeš
2024-01-09 14:31 ` Jeremy Spewock
2024-01-03 22:32 ` [PATCH v6 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps jspewock
2024-01-08 11:52 ` Juraj Linkeš
2024-01-08 16:37 ` Jeremy Spewock
2024-01-03 22:32 ` [PATCH v6 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2024-01-08 12:01 ` Juraj Linkeš
2024-01-08 16:39 ` Jeremy Spewock
2024-01-08 16:40 ` Jeremy Spewock
2024-01-03 22:32 ` [PATCH v6 4/7] dts: add pci addresses to EAL parameters jspewock
2024-01-08 14:59 ` Juraj Linkeš
2024-01-03 22:32 ` [PATCH v6 5/7] dts: allow configuring MTU of ports jspewock
2024-01-08 15:00 ` Juraj Linkeš
2024-01-03 22:32 ` [PATCH v6 6/7] dts: add scatter to the yaml schema jspewock
2024-01-08 15:01 ` Juraj Linkeš
2024-01-03 22:32 ` [PATCH v6 7/7] dts: add pmd_buffer_scatter test suite jspewock
2024-01-08 15:47 ` Juraj Linkeš
2024-01-08 16:53 ` Jeremy Spewock
2024-01-09 15:36 ` [PATCH v7 0/7] dts: Port scatter suite over jspewock
2024-01-09 15:36 ` [PATCH v7 1/7] dts: add startup verification and forwarding modes to testpmd shell jspewock
2024-01-10 13:18 ` Juraj Linkeš
2024-01-10 14:09 ` Jeremy Spewock
2024-01-09 15:36 ` [PATCH v7 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps jspewock
2024-01-09 15:36 ` [PATCH v7 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2024-01-09 15:36 ` [PATCH v7 4/7] dts: add pci addresses to EAL parameters jspewock
2024-01-09 15:36 ` [PATCH v7 5/7] dts: allow configuring MTU of ports jspewock
2024-01-09 15:36 ` [PATCH v7 6/7] dts: add scatter to the yaml schema jspewock
2024-01-09 15:36 ` [PATCH v7 7/7] dts: add pmd_buffer_scatter test suite jspewock
2024-01-10 13:16 ` Juraj Linkeš
2024-01-10 14:09 ` Jeremy Spewock
2024-01-10 13:22 ` [PATCH v7 0/7] dts: Port scatter suite over Juraj Linkeš
2024-01-10 14:42 ` [PATCH v8 " jspewock
2024-01-10 14:42 ` [PATCH v8 1/7] dts: add startup verification and forwarding modes to testpmd shell jspewock
2024-01-10 14:42 ` [PATCH v8 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps jspewock
2024-01-10 14:42 ` [PATCH v8 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2024-01-10 14:42 ` [PATCH v8 4/7] dts: add pci addresses to EAL parameters jspewock
2024-01-10 14:42 ` [PATCH v8 5/7] dts: allow configuring MTU of ports jspewock
2024-01-10 14:42 ` [PATCH v8 6/7] dts: add scatter to the yaml schema jspewock
2024-01-10 14:42 ` [PATCH v8 7/7] dts: add pmd_buffer_scatter test suite jspewock
2024-01-11 10:07 ` [PATCH v8 0/7] dts: Port scatter suite over Juraj Linkeš
2024-02-21 3:34 ` Patrick Robb
2024-03-07 15:00 ` Thomas Monjalon
2024-03-11 14:15 ` Jeremy Spewock
2024-03-11 15:43 ` [PATCH v9 " jspewock
2024-03-11 15:43 ` [PATCH v9 1/7] dts: add startup verification and forwarding modes to testpmd shell jspewock
2024-03-11 15:44 ` [PATCH v9 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps jspewock
2024-03-11 15:44 ` [PATCH v9 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2024-03-11 15:44 ` [PATCH v9 4/7] dts: add pci addresses to EAL parameters jspewock
2024-03-11 15:44 ` [PATCH v9 5/7] dts: allow configuring MTU of ports jspewock
2024-03-11 15:44 ` [PATCH v9 6/7] dts: add scatter to the yaml schema jspewock
2024-03-11 15:44 ` [PATCH v9 7/7] dts: add pmd_buffer_scatter test suite jspewock
2024-03-15 17:41 ` [PATCH v9 0/7] dts: Port scatter suite over Thomas Monjalon
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='CAOb5WZacHRBpBfgLOBY0Br2GYJa9pOu9BMw4TK-g=Fw3pN+pHg@mail.gmail.com' \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=jspewock@iol.unh.edu \
--cc=lylavoie@iol.unh.edu \
--cc=paul.szczepanek@arm.com \
--cc=probb@iol.unh.edu \
--cc=thomas@monjalon.net \
--cc=wathsala.vithanage@arm.com \
--cc=yoan.picchi@foss.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).