Hi Andrew,
thank you for your patches!
On Tue, Sep 02, 2025 at 10:27:24AM +0000, Andrew Bailey wrote:
> diff --git a/dts/framework/remote_session/testpmd_shell.py
> b/dts/framework/remote_session/testpmd_shell.py
> index ad8cb273dc..786714d1c5 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -1278,6 +1278,99 @@ class TestPmdVerbosePacket(TextParser):
> )
>
>
> +class TxOffloadCapability(Flag):
> + """TX offload capabilities of a device.
> +
> + The flags are taken from ``lib/ethdev/rte_ethdev.h``.
> + They're prefixed with ``RTE_ETH_TX_OFFLOAD`` in
> ``lib/ethdev/rte_ethdev.h``
> + instead of ``TX_OFFLOAD``, which is what testpmd changes the prefix to.
> + The values are not contiguous, so the correspondence is preserved
> + by specifying concrete values interspersed between auto() values.
> +
> + The ``TX_OFFLOAD`` prefix has been preserved so that the same flag names
> can be used
> + in :class:`NicCapability`. The prefix is needed in
> :class:`NicCapability` since there's
> + no other qualifier which would sufficiently distinguish it from other
> capabilities.
> +
> + References:
> + DPDK lib: ``lib/ethdev/rte_ethdev.h``
> + testpmd display function:
> ``app/test-pmd/cmdline.c:print_rx_offloads()``
> + """
> +
> + TX_OFFLOAD_VLAN_INSERT = auto()
> + TX_OFFLOAD_IPV4_CKSUM = auto()
> + TX_OFFLOAD_UDP_CKSUM = auto()
> + TX_OFFLOAD_TCP_CKSUM = auto()
> + TX_OFFLOAD_SCTP_CKSUM = auto()
> + TX_OFFLOAD_TCP_TSO = auto()
> + TX_OFFLOAD_UDP_TSO = auto()
> + TX_OFFLOAD_OUTER_IPV4_CKSUM = auto()
> + TX_OFFLOAD_QINQ_INSERT = auto()
> + TX_OFFLOAD_VXLAN_TNL_TSO = auto()
> + TX_OFFLOAD_GRE_TNL_TSO = auto()
> + TX_OFFLOAD_IPIP_TNL_TSO = auto()
> + TX_OFFLOAD_GENEVE_TNL_TSO = auto()
> + TX_OFFLOAD_MACSEC_INSERT = auto()
> + TX_OFFLOAD_MT_LOCKFREE = auto()
> + TX_OFFLOAD_MULTI_SEGS = auto()
> + TX_OFFLOAD_MBUF_FAST_FREE = auto()
> + TX_OFFLOAD_SECURITY = auto()
> + TX_OFFLOAD_UDP_TNL_TSO = auto()
> + TX_OFFLOAD_IP_TNL_TSO = auto()
> + TX_OFFLOAD_OUTER_UDP_CKSUM = auto()
> + TX_OFFLOAD_SEND_ON_TIMESTAMP = auto()
> +
> + @classmethod
> + def from_string(cls, line: str) -> Self:
> + """Make an instance from a string containing the flag names
> separated with a space.
> +
> + Args:
> + line: The line to parse.
> +
> + Returns:
> + A new instance containing all found flags.
> + """
> + flag = cls(0)
> + for flag_name in line.split():
> + flag |= cls[f"TX_OFFLOAD_{flag_name}"]
> + return flag
> +
> + @classmethod
> + def make_parser(cls, per_port: bool) -> ParserFn:
> + """Make a parser function.
> +
> + Args:
> + per_port: If :data:`True`, will return capabilities per port. If
> :data:`False`,
> + will return capabilities per queue.
> +
> + Returns:
> + ParserFn: A dictionary for the `dataclasses.field` metadata
> argument containing a
> + parser function that makes an instance of this flag from
> text.
> + """
> + granularity = "Port" if per_port else "Queue"
> + return TextParser.wrap(
> + TextParser.find(rf"Per {granularity}\s+:(.*)$", re.MULTILINE),
> + cls.from_string,
> + )
The above is creating a lot of duplication. I'd personally implement the
functions in a class that the flags RxOffload and TxOffload can inherit
from. You can deal with `cls[f"TX_OFFLOAD_{flag_name}"]` by introducing
a "PREFIX" ClassVar for all the classes where you can specify it:
PREFIX: ClassVar[str] = "TX_OFFLOAD_"
...
cls[f"{cls.PREFIX}{flag_name}"]
