Re: [PATCH v2 5/5] dts: add testpmd set ports queues

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

On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add a facility to update the number of TX/RX queues during the runtime
> of testpmd.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index ca24b28070..85fbc42696 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> None:
>
>          self.ports_started = True
>
> +    @requires_stopped_ports
> +    def set_ports_queues(self, number_of: int) -> None:

Was there a reason to not add a verify option to this method? It seems
like it could be useful, and the general pattern with testpmd methods
so far is verifying the outcome wherever we can.

> +        """Sets the number of queues per port.
> +
> +        Args:
> +            number_of: The number of RX/TX queues to create per port.

Is there any value in allowing users to set either Rx or Tx rather
than enforcing they change both? I guess I can't think of a specific
reason to do one and not the other off the top of my head, but it
seems like the option could be useful.

> +
> +        Raises:
> +            InternalError: If `number_of` is invalid.

I might be more in favor of this being an
InteractiveCommandExecutionError or some other DTSError just to have a
little more control over the error codes and to make it more clear
that the error came specifically from the framework.

> +        """
> +        if number_of < 1:
> +            raise InternalError("The number of queues must be positive and non-zero")
> +
> +        self.send_command(f"port config all rxq {number_of}")
> +        self.send_command(f"port config all txq {number_of}")
> +
>      def show_port_info_all(self) -> list[TestPmdPort]:
>          """Returns the information of all the ports.
>
> --
> 2.34.1
>