Re: [PATCH v2 2/3] dts: add testpmd methods for test suite

[Nicholas Pratte <npratte@iol.unh.edu>]

On Thu, Jul 11, 2024 at 3:33 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> 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".

Surprised I never saw that. I'll fix this.


> On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@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.

Good catch!

>
> > 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 <npratte@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.

Strange. I can fix that real quick.

>
> >  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."

Yes. This was just a typo. What you have here is what I was going for.

>
> > +
> > +        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
> >