Re: [PATCH v2 1/2] dts: add VXLAN port method to testpmd shell

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

Just a few comments on doc-strings, otherwise:

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

On Fri, Aug 23, 2024 at 4:22 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Add rx_vxlan_port add/rm method to testpmd shell for adding
> or removing a vxlan id to the specified port filter list.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index 43e9f56517..00b75954ef 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -806,6 +806,29 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
>
>          return TestPmdPortStats.parse(output)
>
> +    def rx_vxlan(self, vxlan_id: int, port_id: int, add: bool, verify: bool = True) -> None:
> +        """Add or remove vxlan id to filter list.

It might read better if you replaced "to" here with "to/from".

> +
> +        Args:
> +            vxlan_id: Number of VXLAN ID to add to port filter list.

This is a little nit-picky, but it might be worth removing "Number of"
here and replacing "add to" with "add to/remove from" so it is "VXLAN
ID to add to/remove from port filter list." just so that it reflects
that you can both add and remove using this method.

> +            port_id: Number of port to add VXLAN ID to.

For this line I might be in favor of simplifying it down to something
like "ID of the port to modify VXLAN filter of." That way it doesn't
need all the slashes to account for both adding and removing.

> +            add: If :data:`True`, adds specified VXLAN ID, otherwise removes it.
> +            verify: If :data:`True`, the output of the command is checked to verify
> +                the VXLAN ID was successfully added/removed from the port.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and VXLAN ID
> +                is not successfully added or removed.
> +        """
> +        action = "add" if add else "rm"
> +        vxlan_output = self.send_command(f"rx_vxlan_port {action} {vxlan_id} {port_id}")
> +        if verify:
> +            if "udp tunneling add error" in vxlan_output:
> +                self._logger.debug(f"Failed to set VXLAN:\n{vxlan_output}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to set VXLAN:\n{vxlan_output}"
> +                )
> +
>      def _close(self) -> None:
>          """Overrides :meth:`~.interactive_shell.close`."""
>          self.stop()
> --
> 2.44.0
>