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

[Luca Vizzarro <Luca.Vizzarro@arm.com>]

Great work Jeremy! Just a couple of minor passable improvement points.

On 30/07/2024 14:34, jspewock@iol.unh.edu wrote:

> +@dataclass
> +class TestPmdVerbosePacket(TextParser):
> +    """Packet information provided by verbose output in Testpmd.
> +
> +    The "receive/sent queue" information is not included in this dataclass because this value is
> +    captured on the outer layer of input found in :class:`TestPmdVerboseOutput`.
> +    """
> +
> +    #:
> +    src_mac: str = field(metadata=TextParser.find(r"src=({})".format(REGEX_FOR_MAC_ADDRESS)))
Just a(n optional) nit: TextParser.find(f"src=({REGEX_FOR_MAC_ADDRESS})")
The raw string is only needed to prevent escaping, which we don't do here.
> +    #:
> +    dst_mac: str = field(metadata=TextParser.find(r"dst=({})".format(REGEX_FOR_MAC_ADDRESS)))
As above.
> +    #: Memory pool the packet was handled on.
> +    pool: str = field(metadata=TextParser.find(r"pool=(\S+)"))
> +    #: Packet type in hex.
> +    p_type: int = field(metadata=TextParser.find_int(r"type=(0x[a-fA-F\d]+)"))
> +    #:

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

How about using a regex that matches what you described? ;) Keeping re.S:

    (port \d+/queue \d+.+?ol_flags: [\w ]+)

Would spare you from using complex lookahead constructs and 4.6x less 
steps. Maybe it doesn't work with every scenario? Looks like it works 
well with the sample output I have. Let me know if it works for you.

Best,
Luca