Re: [PATCH v2 5/8] dts: add testpmd shell params

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

This looks good, the only comment I had was in some classes the
docstrings didn't get updated to what was discussed previously in the
comments (making sure the comments are included in the class'
docstring). I tried to point out a few places where I noticed it.
Other than those comments however:

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

On Thu, May 9, 2024 at 7:21 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Implement all the testpmd shell parameters into a data structure.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
>  dts/framework/params/testpmd.py               | 608 ++++++++++++++++++
>  dts/framework/remote_session/testpmd_shell.py |  42 +-
>  dts/tests/TestSuite_pmd_buffer_scatter.py     |   5 +-
>  3 files changed, 615 insertions(+), 40 deletions(-)
>  create mode 100644 dts/framework/params/testpmd.py
>
<snip>
> +
> +
> +class PortTopology(StrEnum):
> +    """Enum representing the port topology."""
> +
> +    paired = auto()
> +    """In paired mode, the forwarding is between pairs of ports, e.g.: (0,1), (2,3), (4,5)."""
> +    chained = auto()
> +    """In chained mode, the forwarding is to the next available port in the port mask, e.g.:
> +    (0,1), (1,2), (2,0).
> +
> +    The ordering of the ports can be changed using the portlist testpmd runtime function.
> +    """
> +    loop = auto()
> +    """In loop mode, ingress traffic is simply transmitted back on the same interface."""
> +

This should likely be the comment style for class vars: `#:`

> +
<snip>
> +
> +@convert_str(hex_from_flag_value)
> +@unique
> +class HairpinMode(Flag):
> +    """Flag representing the hairpin mode."""
> +
> +    TWO_PORTS_LOOP = 1 << 0
> +    """Two hairpin ports loop."""
> +    TWO_PORTS_PAIRED = 1 << 1
> +    """Two hairpin ports paired."""
> +    EXPLICIT_TX_FLOW = 1 << 4
> +    """Explicit Tx flow rule."""
> +    FORCE_RX_QUEUE_MEM_SETTINGS = 1 << 8
> +    """Force memory settings of hairpin RX queue."""
> +    FORCE_TX_QUEUE_MEM_SETTINGS = 1 << 9
> +    """Force memory settings of hairpin TX queue."""
> +    RX_QUEUE_USE_LOCKED_DEVICE_MEMORY = 1 << 12
> +    """Hairpin RX queues will use locked device memory."""
> +    RX_QUEUE_USE_RTE_MEMORY = 1 << 13
> +    """Hairpin RX queues will use RTE memory."""
> +    TX_QUEUE_USE_LOCKED_DEVICE_MEMORY = 1 << 16
> +    """Hairpin TX queues will use locked device memory."""
> +    TX_QUEUE_USE_RTE_MEMORY = 1 << 18
> +    """Hairpin TX queues will use RTE memory."""
> +

Same thing in this class, these should likely be documented as class
vars with `#:`

> +
<snip>
> +class SimpleMempoolAllocationMode(StrEnum):
> +    """Enum representing simple mempool allocation modes."""
> +
> +    native = auto()
> +    """Create and populate mempool using native DPDK memory."""
> +    xmem = auto()
> +    """Create and populate mempool using externally and anonymously allocated area."""
> +    xmemhuge = auto()
> +    """Create and populate mempool using externally and anonymously allocated hugepage area."""
> +

Also here. Same as the previous, should likely be `#:`

> +
> +@dataclass(kw_only=True)
<snip>
> 2.34.1
>