Re: [PATCH v3 2/3] dts: add functions to testpmd shell

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

On Wed, Jun 26, 2024 at 9:52 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> added set promisc, set verbose, and port stop
> commands to testpmd shell.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 157 +++++++++++++++---
>  1 file changed, 136 insertions(+), 21 deletions(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index ec22f72221..a1928b2cec 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -645,27 +645,6 @@ def start(self, verify: bool = True) -> None:
>                          "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
> -            ):
> -                self._logger.debug(f"Failed to stop packet forwarding: \n{stop_cmd_output}")
> -                raise InteractiveCommandExecutionError("Testpmd failed to stop packet forwarding.")
> -

Apologies, I didn't mean to remove the entire method from testpmd
shell, I just mean the changes that were made to it in the last
version. This method is still required since the scatter test suite
uses it on main currently. The version that is on main should be fine
though.

>      def get_devices(self) -> list[TestPmdDevice]:
>          """Get a list of device names that are known to testpmd.
>
<snip>
> +    def set_promisc_on(self, port: int, on: bool, verify: bool = True):

I had some comments on this method in your VLAN series that still
apply here. Seems like this method if needed in quite a few places,
definitely good we're getting it added.

> +        """Turns promiscuous mode on/off for the specified port
> +
> +        Args:
> +            port: port number to use, should be within 0-32.
> +            on: if :data:`True`, turn promisc mode on, otherwise turn off.
> +            verify: if :data:`True` an additional command will be sent to verify that promisc mode
> +                is properly set. Defaults to :data:`True`.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and promisc mode
> +            is not correctly set.
> +        """
> +        if on:
> +            promisc_output = self.send_command(f"set promisc {port} on")
> +        else:
> +            promisc_output = self.send_command(f"set promisc {port} off")
> +        if verify:
> +            if (on and "Promiscuous mode: enabled" not in
> +            self.send_command(f"show port info {port}")):
> +                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
> +            elif (not on and "Promiscuous mode: disabled" not in
> +            self.send_command(f"show port info {port}")):
> +                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
> +
> +
> +    def set_verbose(self, level: int, verify: bool = True):
> +        """Set debug verbosity level.
> +
> +        Args:
> +            level: 0 - silent except for error
> +            1 - fully verbose except for Tx packets
> +            2 - fully verbose except for Rx packets
> +            >2 - fully verbose

I think in the VLAN suite series these lines got indented, we should
probably also update this method to match what is there.



> +            verify: if :data:`True` an additional command will be sent to verify that verbose level
> +                is properly set. Defaults to :data:`True`.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
> +            is not correctly set.
> +        """
> +        verbose_output = self.send_command(f"set verbose {level}")
> +        if verify:
> +            if "Change verbose level" not in verbose_output:
> +                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
> +
>      def close(self) -> None:
>          """Overrides :meth:`~.interactive_shell.close`."""
>          self.send_command("quit", "")
> --
> 2.44.0
>