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,
ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru,
dev@dpdk.org
Subject: Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps
Date: Thu, 16 Nov 2023 19:52:52 +0100 [thread overview]
Message-ID: <CAOb5WZZgSpE=kiuU=NQXbP9+sjy+U7nRo+bL1pSYQvT_c38d=w@mail.gmail.com> (raw)
In-Reply-To: <20231113202833.12900-5-jspewock@iol.unh.edu>
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Modified interactive applications to allow for the ability to pass
> parameters into the app on start up. Also modified the way EAL
> parameters are handled so that the trailing "--" separator is added be
> default after all EAL parameters.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/remote_session/remote/testpmd_shell.py | 2 +-
> dts/framework/testbed_model/sut_node.py | 11 +++++++----
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
> index 3ea16c7ab3..3f6a86aa35 100644
> --- a/dts/framework/remote_session/remote/testpmd_shell.py
> +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> @@ -32,7 +32,7 @@ def _start_application(
> self, get_privileged_command: Callable[[str], str] | None
> ) -> None:
> """See "_start_application" in InteractiveShell."""
> - self._app_args += " -- -i"
> + self._app_args += " -i"
> super()._start_application(get_privileged_command)
>
> def get_devices(self) -> list[TestPmdDevice]:
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index 4161d3a4d5..bcac939e72 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -377,7 +377,8 @@ def create_interactive_shell(
> shell_cls: Type[InteractiveShellType],
> timeout: float = SETTINGS.timeout,
> privileged: bool = False,
> - eal_parameters: EalParameters | str | None = None,
> + eal_parameters: EalParameters | str = "",
I think it makes more sense if the type is EalParameters | None. With
this change, the str type of eal_parameters moved to app_parameters.
> + app_parameters: str = "",
> ) -> InteractiveShellType:
> """Factory method for creating a handler for an interactive session.
>
> @@ -392,12 +393,14 @@ def create_interactive_shell(
> eal_parameters: List of EAL parameters to use to launch the app. If this
> isn't provided or an empty string is passed, it will default to calling
> create_eal_parameters().
> + app_parameters: Additional arguments to pass into the application on the
> + command-line.
> Returns:
> Instance of the desired interactive application.
> """
> - if not eal_parameters:
> + if not eal_parameters and shell_cls.dpdk_app:
> eal_parameters = self.create_eal_parameters()
> -
> + eal_parameters = f"{eal_parameters} --"
I think this change is more complicated than it seems at first glance.
Before we either passed EalParameters (meant for DPDK apps) or a
string (meant for other apps). There was no additional check for these
assumptions.
Now that we've split it, I see some cases which seem to be not covered.
1. The app is a DPDK app and we pass EalParameters. The two hyphens
are not added.
2. The app is not a DPDK app and we pass EalParameters. Similarly to
current behavior (without the patch), we kinda assume that the caller
won't do this, but now that we're modifying the behavior, let's add a
check that won't allow EalParameters with non-DPDK apps.
> # We need to append the build directory for DPDK apps
> if shell_cls.dpdk_app:
> shell_cls.path = self.main_session.join_remote_path(
> @@ -405,7 +408,7 @@ def create_interactive_shell(
> )
>
> return super().create_interactive_shell(
> - shell_cls, timeout, privileged, str(eal_parameters)
> + shell_cls, timeout, privileged, f"{eal_parameters} {app_parameters}"
> )
>
> def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> --
> 2.42.0
>
next prev parent reply other threads:[~2023-11-16 18:53 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
2023-11-13 20:28 ` [PATCH v3 1/7] dts: Add scatter test suite jspewock
2023-11-15 7:04 ` Patrick Robb
2023-11-16 19:20 ` Juraj Linkeš
2023-11-21 19:26 ` Jeremy Spewock
2023-11-22 8:47 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 2/7] dts: add waiting for port up in testpmd jspewock
2023-11-16 19:05 ` Juraj Linkeš
2023-11-17 18:09 ` Jeremy Spewock
2023-11-13 20:28 ` [PATCH v3 3/7] dts: add scatter to the yaml schema jspewock
2023-11-13 20:28 ` [PATCH v3 4/7] dts: allow passing parameters into interactive apps jspewock
2023-11-16 18:52 ` Juraj Linkeš [this message]
2023-11-17 18:08 ` Jeremy Spewock
2023-11-20 14:35 ` Juraj Linkeš
2023-11-21 21:55 ` Jeremy Spewock
2023-11-13 20:28 ` [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer jspewock
2023-11-16 18:34 ` Juraj Linkeš
2023-11-17 18:05 ` Jeremy Spewock
2023-11-20 14:31 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 6/7] dts: add pci addresses to EAL parameters jspewock
2023-11-16 18:10 ` Juraj Linkeš
2023-11-17 17:13 ` Jeremy Spewock
2023-11-13 20:28 ` [PATCH v3 7/7] dts: allow configuring MTU of ports jspewock
2023-11-16 18:05 ` Juraj Linkeš
2023-11-17 17:06 ` Jeremy Spewock
2023-11-16 19:23 ` [PATCH v3 0/7] dts: Port scatter suite over Juraj Linkeš
2023-12-14 22:10 ` [PATCH v4 " jspewock
2023-12-14 22:10 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
2023-12-14 22:10 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
2023-12-14 22:10 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2023-12-14 22:10 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
2023-12-14 22:10 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
2023-12-14 22:10 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
2023-12-14 22:10 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
2023-12-18 17:22 ` [PATCH v4 0/7] dts: Port scatter suite over jspewock
2023-12-18 17:22 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
2023-12-18 17:22 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
2023-12-18 17:22 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
2023-12-18 17:22 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
2023-12-18 17:22 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
2023-12-18 17:22 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
2023-12-18 17:22 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
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='CAOb5WZZgSpE=kiuU=NQXbP9+sjy+U7nRo+bL1pSYQvT_c38d=w@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=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).