Re: [PATCH v3 1/1] dts: add methods for modifying MTU to testpmd shell

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

On Fri, Sep 6, 2024 at 9:58 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> > index ca24b28070..c1462ba2d3 100644
> > --- a/dts/framework/remote_session/testpmd_shell.py
> > +++ b/dts/framework/remote_session/testpmd_shell.py
> > @@ -888,6 +888,51 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
>
> > +    def set_port_mtu_all(self, mtu: int, verify: bool = True) -> None:
> > +        """Change the MTU of all ports using testpmd.
> > +
> > +        Runs :meth:`set_port_mtu` for every port that testpmd is aware of.
> > +
> > +        Args:
> > +            mtu: Desired value for the MTU to be set to.
> > +            verify: Whether to verify that setting the MTU on each port was successful or not.
> > +                Defaults to :data:`True`.
> > +
> > +        Raises:
> > +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the MTU was not
> > +                properly updated on at least one port.
> > +        """
> > +        if self._app_params.ports is not None:
>
> We should utilize the port info caching patch here:
> https://patches.dpdk.org/project/dpdk/patch/20240823074137.13989-1-juraj.linkes@pantheon.tech/
>
> Other than that, the patch looks good.
>

That's a good idea, I also like that it sort of detaches this method
from the subtle requirement that testpmd is started with an allow list
of ports. This requirement is enforced right now, but I think it makes
more sense for this method to operate based on what testpmd is aware
of anyway. It does add another patch to the dependency chain though,
we should probably prioritize getting the info caching patch into
next-dts.

> > +            for port_id in range(len(self._app_params.ports)):
> > +                self.set_port_mtu(port_id, mtu, verify)
> > +
> >       def _close(self) -> None:
> >           """Overrides :meth:`~.interactive_shell.close`."""
> >           self.stop()
>