On Thu, Sep 26, 2024 at 4:25 AM Juraj Linkeš <juraj.lin...@pantheon.tech> wrote: > > > > diff --git a/dts/framework/remote_session/testpmd_shell.py > > b/dts/framework/remote_session/testpmd_shell.py > > > @@ -581,6 +581,506 @@ class TestPmdPortStats(TextParser): > > tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)")) > > > > > > +class PacketOffloadFlag(Flag): > > > + #: RX IEEE1588 L2 Ethernet PT Packet. > > + RTE_MBUF_F_RX_IEEE1588_PTP = 1 << 9 > > + #: RX IEEE1588 L2/L4 timestamped packet. > > + RTE_MBUF_F_RX_IEEE1588_TMST = 1 << 10 > > There are a few instances with two or three consecutive bits set > expliticly instead with auto(). I don't know if it's better to use > auto() or the explitic value, just wanted to point it out. >
If it was only two consecutive bits I avoided using auto because I thought that made it more clear (at least for me, it felt like less bouncing back and forth). This is a good point though, I also don't know which is really better. > > > + > > + #: FD id reported if FDIR match. > > + RTE_MBUF_F_RX_FDIR_ID = 1 << 13 > > + #: Flexible bytes reported if FDIR match. > > + RTE_MBUF_F_RX_FDIR_FLX = 1 << 14 > > + @classmethod > > + def from_str(cls, flags: str) -> Self: > > Now that we're doing the same thing as the other classes, I think it > makes sense to just flat out copy-paste the from_list_string method. Sure, that makes sense to me. > > > + """Makes an instance from a string containing whitespace-separated > > the flag members. > > + > > + Args: > > + arr: A string containing ol_flag values. > > + > > + Returns: > > + A new instance of the flag. > > + """ > > + flag = cls(0) > > + for name in flags.split(): > > + if hasattr(cls, name): > > This is still different from the other class. I think making these > exactly the same would make it clear what needs to be put into the base > class if we ever create one. Ack. > > > + flag |= cls[name] > > + return flag > > > + > > +class RtePTypes(Flag): > > + """Flag representing possible packet types in DPDK verbose output. > > + > > + Values in this class are derived from definitions in the RTE MBUF > > ptype library in DPDK located > > + in lib/mbuf/rte_mbuf_ptype.h. Specifically, the names of values in > > this class should match the > > + possible return options from the methods rte_get_ptype_*_name in > > rte_mbuf_ptype.c. > > I think these are functions (rte_get_ptype_*_name), not methods. > Ahh, good call. I wasn't thinking of the distinction when I wrote it. > > + """ > > You didn't update the docstring here (double backticks (for file and > function names) and the References: section). Good catch, I'll fix those as well. > > > > + @classmethod > > + def from_str(cls, flags: str) -> Self: > > The same comments apply here. Ack. > >
