There were a few emails that came through for this series but this was
the most recent one so I went with reviewing this one, but it looks
like the more descriptive commit subject got lost in this iteration.
It would probably be worth adding that back.

Additionally, it looks like the functions you added here (including
the ones added from the VLAN suite, but I believe those ones are
already fixed in the latest version ) are all missing their return
types. Of course they don't return anything, but that is still useful
to note by annotating it as "None".

On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npra...@iol.unh.edu> wrote:
>
> Several 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
>  - rx_vlan_add
>  - rx_vlan_rm
>  - vlan_filter_set_on
>  - vlan_filter_set_off
>  - set_promisc
>
> 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 booling 'add or remove' parameter. The success or

I think this is a typo and "booling" should be "boolean" but I could be wrong.

> failure of each call can be verified if a user deems it necessary.
>
> The other methods listed are implemented in other respective test
> suites, and their implementations have been copied, but are subject to
> change; they are not the focus of this patch.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npra...@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 177 ++++++++++++++++++
>  dts/tests/TestSuite_mac_filter.py             |   0

It looks like creating the file somehow snuck into the diff for this
commit instead of the other one that populates it.

>  2 files changed, 177 insertions(+)
>  create mode 100644 dts/tests/TestSuite_mac_filter.py
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py 
> b/dts/framework/remote_session/testpmd_shell.py
> index ec22f72221..0be1fb8754 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
<snip>
> +    def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: 
> bool, verify: bool = True):
> +        """Add or remove multicast mac address to a specified port filter.

Just to make this more clear that you specify the port and not the
port filter, it might be helpful here to show that the port possesses
the filter by saying "a specified port's filter."

> +
> +        Args:
> +            port_id: The port ID the multicast address is set on.
> +            multi_addr: The multicast address to be added to 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}"
> +                )
<snip>

>      def show_port_stats_all(self) -> list[TestPmdPortStats]:
>          """Returns the statistics of all the ports.
>
> diff --git a/dts/tests/TestSuite_mac_filter.py 
> b/dts/tests/TestSuite_mac_filter.py
> new file mode 100644
> index 0000000000..e69de29bb2
> --
> 2.44.0
>

Reply via email to