On Thu, Jul 11, 2024 at 3:33 PM Jeremy Spewock <jspew...@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 <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.
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 <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.
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
> >
