Re: [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses

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

Oops, I just reviewed an older version without realizing this one
existed, haha. Although, seeing the full diff again I saw a few
documentation nit-picks that I put below.

On Fri, Jul 26, 2024 at 12:45 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> New methods have been added to TestPMDShell in order to produce the mac
> filter's individual test cases:
>  - set_mac_addr
>  - set_multicast_mac_addr
>
> set_mac_addr and set_multicast_addr were created for the mac filter test
> suite, enabling users to both add or remove mac and multicast
> addresses based on a boolean 'add or remove' parameter. The success or
> failure of each call can be verified if a user deems it necessary.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index 8e5a1c084a..64ffb23439 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -765,6 +765,65 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
>
>          return TestPmdPort.parse(output)
>
> +    def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: bool = True) -> None:
> +        """Add or remove a mac address on a given port's Allowlist.
> +
> +        Args:
> +            port_id: The port ID the mac address is set on.
> +            mac_address: The mac address to be added or removed to the specified port.

This might be more clear if it said the mac address to be "added to or
removed from" since the "removed to" doesn't exactly fit.

> +            add: If :data:`True`, add the specified mac address. If :data:`False`, remove specified
> +                mac address.
> +            verify: If :data:'True', assert that the 'mac_addr' operation was successful. If
> +                :data:'False', run the command and skip this assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If the set mac address operation fails.
> +        """
> +        mac_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid argument provided to mac_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument provided")
> +
> +        if verify:
> +            if "mac_addr_cmd error:" in output:
> +                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mac_cmd} {mac_address} on port {port_id} \n{output}"
> +                )
> +
> +    def set_multicast_mac_addr(
> +        self, port_id: int, multi_addr: str, add: bool, verify: bool = True
> +    ) -> None:
> +        """Add or remove multicast mac address to a specified port's filter.

Same thing here as above, but it might be a little trickier here.
Maybe this could be something like allow or disallow to make it more
homogeneous? I'm not quite sure.

> +
> +        Args:
> +            port_id: The port ID the multicast address is set on.
> +            multi_addr: The multicast address to be added to the filter.

Since this method also has an `add` parameter, it would probably be
helpful to specify that this also can be either added to or removed
from the filter.


> +            add: If :data:'True', add the specified multicast address to the port filter.
> +                If :data:'False', remove the specified multicast address from the port filter.
> +            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
> +                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
> +        """
> +        mcast_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid arguments provided to mcast_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument provided")
> +
> +        if verify:
> +            if (
> +                "Invalid multicast_addr" in output
> +                or f'multicast address {"already" if add else "not"} filtered by port' in output
> +            ):
> +                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
> +                )
> +
>      def show_port_stats_all(self) -> list[TestPmdPortStats]:
>          """Returns the statistics of all the ports.
>
> --
> 2.44.0
>