Re: [PATCH v2 1/1] dts: add text parser for testpmd verbose output

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

On Tue, Jul 30, 2024 at 11:41 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Good work! I left some probing/clarifying comments below for you.
>
> <snip>
> > +@dataclass
> > +class TestPmdVerboseOutput(TextParser):
> > +    """Verbose output generated by Testpmd.
> > +
> > +    This class is the top level of the output, containing verbose output delimited by
> > +    "port X/queue Y: sent/received Z packets".
> > +    """
> > +
> > +    #: ID of the port that handled the packet.
> > +    port_id: int = field(metadata=TextParser.find_int(r"port (\d+)/queue \d+"))
> > +    #: ID of the queue that handled the packet.
> > +    queue_id: int = field(metadata=TextParser.find_int(r"port \d+/queue (\d+)"))
> > +    #: Whether the packet was received or sent by the queue/port.
> > +    was_received: bool = field(metadata=TextParser.find(r"received \d+ packets"))
> > +    #: List of packets handed by the port/queue in this section.
> > +    packets: list[TestPmdVerbosePacket] = field(
> > +        metadata=TextParser.wrap(
> > +            TextParser.find_all(r"(src=[\w\s=:-]+ol_flags: [\w ]+)"),
> > +            lambda matches_arr: list(map(TestPmdVerbosePacket.parse, matches_arr)),
> > +        )
> > +    )
> > +
> > +
> >  class TestPmdShell(DPDKShell):
> >      """Testpmd interactive shell.
> >
> > @@ -645,7 +767,7 @@ def start(self, verify: bool = True) -> None:
> >                          "Not all ports came up after starting packet forwarding in testpmd."
> >                      )
> >
> > -    def stop(self, verify: bool = True) -> None:
> > +    def stop(self, verify: bool = True) -> str:
> >          """Stop packet forwarding.
> >
> >          Args:
> > @@ -656,6 +778,9 @@ def stop(self, verify: bool = True) -> None:
> >          Raises:
> >              InteractiveCommandExecutionError: If `verify` is :data:`True` and the command to stop
> >                  forwarding results in an error.
> > +
> > +        Returns:
> > +            Output gathered from sending the stop command.
> >          """
> >          stop_cmd_output = self.send_command("stop")
> >          if verify:
> > @@ -665,6 +790,7 @@ def stop(self, verify: bool = True) -> None:
> >              ):
> >                  self._logger.debug(f"Failed to stop packet forwarding: \n{stop_cmd_output}")
> >                  raise InteractiveCommandExecutionError("Testpmd failed to stop packet forwarding.")
> > +        return stop_cmd_output
> <snip>
> >
> > +    @staticmethod
> > +    def extract_verbose_output(output: str) -> list[TestPmdVerboseOutput]:
> > +        """Extract the verbose information present in given testpmd output.
> > +
> > +        This method extracts sections of verbose output that begin with the line
> > +        "port X/queue Y: sent/received Z packets" and end with the ol_flags of a packet.
> > +
> > +        Args:
> > +            output: Testpmd output that contains verbose information
> > +
> > +        Returns:
> > +            List of parsed packet information gathered from verbose information in `output`.
> > +        """
> > +        iter = re.finditer(r"(port \d+/queue \d+:.*?(?=port \d+/queue \d+|$))", output, re.S)
> > +        return [TestPmdVerboseOutput.parse(s.group(0)) for s in iter]
> > +
>
> I'm trying to think of circumstances where we as developers would be
> looking for anything else besides verbose output from the stop method.
> Running the command outputs some statistics, but information like port
> stats is displayed after the stop command is initiated. I'm
> implementing this system right now for one of my test suites, and I'm
> wondering if there might be any feasible way to extract output without
> needing to input any explicit outputs into this method. I'm putting
> output = testpmd.stop() and then calling this method. It looks
> something like this:
>
> verbose_output = testpmd.extract_verbose_output(testpmd.stop())
>
> This is easy enough, but it might be a bit confusing for someone new
> to the framework. The way that output is gathered is still elusive to
> me, and I'm guessing that any commands run in-between setting verbose
> mode and when you stop testpmd might influence how output is
> generated. But in my experience so far, any statistics or information
> I need is gathered before packets are sent, and the need for verbose
> output in test cases is one after the other (i send a packet, look at
> verbose output, and then move onto the next packet).

You're right that in most cases it would come from the stop output,
but the output from that stop command has one other thing as well that
I would consider valuable which is statistics of packets handled by
ports specifically for the duration of the packet forwarding you are
stopping. It is also true that sending other testpmd commands while
verbose output is being sent will change what is collected, so I
didn't want to tie the method specifically to the stop command since
if you did a start command then checked port statistics for example,
it would consume all of the verbose output up until the command to
check port statistics.

For the reason stated above I think it actually might make sense to
make it so that every testpmd method (including ones that currently
return dataclasses) return their original, unmodified collected output
from the testpmd shell as well. Things like port stats can return both
in a tuple potentially. This way if there is asynchronous output like
there is with verbose output other commands don't completely remove
it.

> <snip>