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 <jspew...@iol.unh.edu> On Thu, May 9, 2024 at 7:21 AM Luca Vizzarro <luca.vizza...@arm.com> wrote: > > Implement all the testpmd shell parameters into a data structure. > > Signed-off-by: Luca Vizzarro <luca.vizza...@arm.com> > Reviewed-by: Paul Szczepanek <paul.szczepa...@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 >