diff mbox series

[v6,2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps

Message ID 20240103223206.23129-3-jspewock@iol.unh.edu (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dts: Port scatter suite over |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jeremy Spewock Jan. 3, 2024, 10:32 p.m. UTC 
  From: Jeremy Spewock <jspewock@iol.unh.edu>

Changed the factory method for creating interactive apps in the SUT Node
so that EAL parameters would only be passed into DPDK apps since
non-DPDK apps wouldn't be able to process them. Also modified
interactive apps to allow for the ability to pass parameters into the
app on startup so that the applications can be started with certain
configuration steps passed on the command line.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---

I ended up reverting part of this back to making the argument for
eal_parameters allowed to be a string. This was because it was casuing
mypy errors where the method signatures of sut_node did not match with
that of node.

 dts/framework/remote_session/testpmd_shell.py |  2 +-
 dts/framework/testbed_model/sut_node.py       | 14 +++++++++-----
 2 files changed, 10 insertions(+), 6 deletions(-)

Comments

Juraj Linkeš Jan. 8, 2024, 11:52 a.m. UTC | #1 
On Wed, Jan 3, 2024 at 11:33 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Changed the factory method for creating interactive apps in the SUT Node
> so that EAL parameters would only be passed into DPDK apps since
> non-DPDK apps wouldn't be able to process them. Also modified
> interactive apps to allow for the ability to pass parameters into the
> app on startup so that the applications can be started with certain
> configuration steps passed on the command line.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>
> I ended up reverting part of this back to making the argument for
> eal_parameters allowed to be a string. This was because it was casuing
> mypy errors where the method signatures of sut_node did not match with
> that of node.
>

This is because the signatures don't actually match :-)

The eal_parameters parameter is added on not top of what's in the base
methods. I suggest we move eal_parameters to the end and then we don't
need to allow str for eal_parameters.

>  dts/framework/remote_session/testpmd_shell.py |  2 +-
>  dts/framework/testbed_model/sut_node.py       | 14 +++++++++-----
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index f310705fac..8f40e8f40e 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -118,7 +118,7 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None
>          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._app_args += " -i --mask-event intr_lsc"
>          self.number_of_ports = self._app_args.count("-a ")
>          super()._start_application(get_privileged_command)
>
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index c4acea38d1..4df18bc183 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -431,6 +431,7 @@ def create_interactive_shell(
>          timeout: float = SETTINGS.timeout,
>          privileged: bool = False,
>          eal_parameters: EalParameters | str | None = None,
> +        app_parameters: str = "",

What I meant above is we should move app_parameters before
eal_parameters and then we can remove the str type of eal_parameters.

>      ) -> InteractiveShellType:
>          """Extend the factory for interactive session handlers.
>
> @@ -449,20 +450,23 @@ 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
>                  :meth:`create_eal_parameters`.
> +            app_parameters: Additional arguments to pass into the application on the
> +                command-line.
>
>          Returns:
>              An instance of the desired interactive application shell.
>          """
> -        if not eal_parameters:
> -            eal_parameters = self.create_eal_parameters()
> -
> -        # We need to append the build directory for DPDK apps
> +        # We need to append the build directory and add EAL parameters for DPDK apps
>          if shell_cls.dpdk_app:
> +            if not eal_parameters:
> +                eal_parameters = self.create_eal_parameters()
> +            app_parameters = f"{eal_parameters} -- {app_parameters}"
> +
>              shell_cls.path = self.main_session.join_remote_path(
>                  self.remote_dpdk_build_dir, shell_cls.path
>              )
>
> -        return super().create_interactive_shell(shell_cls, timeout, privileged, str(eal_parameters))
> +        return super().create_interactive_shell(shell_cls, timeout, privileged, app_parameters)
>
>      def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
>          """Bind all ports on the SUT to a driver.
> --
> 2.43.0
>
Jeremy Spewock Jan. 8, 2024, 4:37 p.m. UTC | #2 
On Mon, Jan 8, 2024 at 6:52 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> On Wed, Jan 3, 2024 at 11:33 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Changed the factory method for creating interactive apps in the SUT Node
> > so that EAL parameters would only be passed into DPDK apps since
> > non-DPDK apps wouldn't be able to process them. Also modified
> > interactive apps to allow for the ability to pass parameters into the
> > app on startup so that the applications can be started with certain
> > configuration steps passed on the command line.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> >
> > I ended up reverting part of this back to making the argument for
> > eal_parameters allowed to be a string. This was because it was casuing
> > mypy errors where the method signatures of sut_node did not match with
> > that of node.
> >
>
> This is because the signatures don't actually match :-)
>
> The eal_parameters parameter is added on not top of what's in the base
> methods. I suggest we move eal_parameters to the end and then we don't
> need to allow str for eal_parameters.
>
> >  dts/framework/remote_session/testpmd_shell.py |  2 +-
> >  dts/framework/testbed_model/sut_node.py       | 14 +++++++++-----
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/dts/framework/remote_session/testpmd_shell.py
> b/dts/framework/remote_session/testpmd_shell.py
> > index f310705fac..8f40e8f40e 100644
> > --- a/dts/framework/remote_session/testpmd_shell.py
> > +++ b/dts/framework/remote_session/testpmd_shell.py
> > @@ -118,7 +118,7 @@ def _start_application(self, get_privileged_command:
> Callable[[str], str] | None
> >          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._app_args += " -i --mask-event intr_lsc"
> >          self.number_of_ports = self._app_args.count("-a ")
> >          super()._start_application(get_privileged_command)
> >
> > diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> > index c4acea38d1..4df18bc183 100644
> > --- a/dts/framework/testbed_model/sut_node.py
> > +++ b/dts/framework/testbed_model/sut_node.py
> > @@ -431,6 +431,7 @@ def create_interactive_shell(
> >          timeout: float = SETTINGS.timeout,
> >          privileged: bool = False,
> >          eal_parameters: EalParameters | str | None = None,
> > +        app_parameters: str = "",
>
> What I meant above is we should move app_parameters before
> eal_parameters and then we can remove the str type of eal_parameters.
>

Sounds good to me, I'll move these around in the next version.


>
> >      ) -> InteractiveShellType:
> >          """Extend the factory for interactive session handlers.
> >
> > @@ -449,20 +450,23 @@ 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
> >                  :meth:`create_eal_parameters`.
> > +            app_parameters: Additional arguments to pass into the
> application on the
> > +                command-line.
> >
> >          Returns:
> >              An instance of the desired interactive application shell.
> >          """
> > -        if not eal_parameters:
> > -            eal_parameters = self.create_eal_parameters()
> > -
> > -        # We need to append the build directory for DPDK apps
> > +        # We need to append the build directory and add EAL parameters
> for DPDK apps
> >          if shell_cls.dpdk_app:
> > +            if not eal_parameters:
> > +                eal_parameters = self.create_eal_parameters()
> > +            app_parameters = f"{eal_parameters} -- {app_parameters}"
> > +
> >              shell_cls.path = self.main_session.join_remote_path(
> >                  self.remote_dpdk_build_dir, shell_cls.path
> >              )
> >
> > -        return super().create_interactive_shell(shell_cls, timeout,
> privileged, str(eal_parameters))
> > +        return super().create_interactive_shell(shell_cls, timeout,
> privileged, app_parameters)
> >
> >      def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> >          """Bind all ports on the SUT to a driver.
> > --
> > 2.43.0
> >
>
diff mbox series

Patch

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index f310705fac..8f40e8f40e 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -118,7 +118,7 @@  def _start_application(self, get_privileged_command: Callable[[str], str] | None
         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._app_args += " -i --mask-event intr_lsc"
         self.number_of_ports = self._app_args.count("-a ")
         super()._start_application(get_privileged_command)
 
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index c4acea38d1..4df18bc183 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -431,6 +431,7 @@  def create_interactive_shell(
         timeout: float = SETTINGS.timeout,
         privileged: bool = False,
         eal_parameters: EalParameters | str | None = None,
+        app_parameters: str = "",
     ) -> InteractiveShellType:
         """Extend the factory for interactive session handlers.
 
@@ -449,20 +450,23 @@  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
                 :meth:`create_eal_parameters`.
+            app_parameters: Additional arguments to pass into the application on the
+                command-line.
 
         Returns:
             An instance of the desired interactive application shell.
         """
-        if not eal_parameters:
-            eal_parameters = self.create_eal_parameters()
-
-        # We need to append the build directory for DPDK apps
+        # We need to append the build directory and add EAL parameters for DPDK apps
         if shell_cls.dpdk_app:
+            if not eal_parameters:
+                eal_parameters = self.create_eal_parameters()
+            app_parameters = f"{eal_parameters} -- {app_parameters}"
+
             shell_cls.path = self.main_session.join_remote_path(
                 self.remote_dpdk_build_dir, shell_cls.path
             )
 
-        return super().create_interactive_shell(shell_cls, timeout, privileged, str(eal_parameters))
+        return super().create_interactive_shell(shell_cls, timeout, privileged, app_parameters)
 
     def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the SUT to a driver.