Re: [PATCH v2 1/4] dts: improve starting and stopping interactive shells

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

On Fri, May 31, 2024 at 12:37 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> On 30/05/2024 17:33, jspewock@iol.unh.edu wrote:
> > @@ -93,17 +102,39 @@ def __init__(
> >       def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
> >           """Starts a new interactive application based on the path to the app.
> >
> > -        This method is often overridden by subclasses as their process for
> > -        starting may look different.
> > +        This method is often overridden by subclasses as their process for starting may look
> > +        different. Initialization of the shell on the host can be retried up to 5 times. This is
> > +        done because some DPDK applications need slightly more time after exiting their script to
> > +        clean up EAL before others can start.
> > +
> > +        When the application is started we also bind a class for finalization to this instance of
> > +        the shell to ensure proper cleanup of the application.
> >
> >           Args:
> >               get_privileged_command: A function (but could be any callable) that produces
> >                   the version of the command with elevated privileges.
> >           """
> > +        self._finalizer = weakref.finalize(self, self._close)
> > +        max_retries = 5
> > +        self._ssh_channel.settimeout(1)
>
> This timeout being too short is causing the testpmd shell (which is
> still loading in my case) to be spammed with testpmd instantiation
> commands. Unfortunately causing the next commands to fail too, as the
> testpmd shell has received a lot of garbage.
>
> 5 seconds of timeout seemed to have worked just fine in my case.

Ack.
It's hard to gauge exactly how long this timeout should be since it
will be different between every system (and between different NICs on
the same system) but I think 5 seconds should be a reasonable amount
of time for testpmd to start in most cases. The only blanket way I
could see to fix this would be to just clean out the buffer before
sending every command so that the previous command outputting harmless
clutter in the buffer doesn't cause every future command sent into
that shell to also break. This idea would likely slow things down
however and doesn't fit the scope of this patch regardless.


>
> >           start_command = f"{self.path} {self._app_args}"
> >           if get_privileged_command is not None:
> >               start_command = get_privileged_command(start_command)
>