From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6342D45949; Mon, 9 Sep 2024 13:44:15 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 05E8F402BB; Mon, 9 Sep 2024 13:44:15 +0200 (CEST) Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by mails.dpdk.org (Postfix) with ESMTP id DA55B40299 for ; Mon, 9 Sep 2024 13:44:12 +0200 (CEST) Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-a8d404c7634so163648966b.3 for ; Mon, 09 Sep 2024 04:44:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1725882251; x=1726487051; darn=dpdk.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=EPLW/ca9ws8Uqlx6IAjgyg1tUSBwDpQAyIx0o5SDefA=; b=OB6QSFwM/A0+vsLhORFX45NGNJ6KrZnpqVbwF1KJcbEPrQfPB/J4UrIc6c/Qt9Fcvd jiWQAXACm2/YvXv4sm/MBtTRT3KwFIs3xokHU/zf41+IA2aFRYuYXiiVBBOv6I7EBFnA d2sJk03cO4vnm81r9PDQANhd7UBAajF3vJcJI4MflA7AHIygXP8m8OP8ghrzcgn7hAdp WmDZGQNhRIjBNCM34suDNKlZ3H/KbEykm30OwYJ2wxuaJ6Mwbw8lAIaY9weJN3pGzXLi RbOs5J/oR6rPCzmMCXDTE32PfVkkWqHo2bXhptSLz/cpi9nz0nsoxW7VgNWAfZDJmlUZ Vmpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725882251; x=1726487051; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=EPLW/ca9ws8Uqlx6IAjgyg1tUSBwDpQAyIx0o5SDefA=; b=nuRvoWxxdrFgAg5jkwkGonnj88OBx0Nl6bNgpiReD9EA3V4SuzmLQNxi0lB602zjiq s3p1GSRGIEb1HESoaxHHSD732LxcUu8Zkv4o05ZCJkbZC/BaxPJHotTQFig17X1IeK/g 1OQY+qPh0jq7So0Nb50xslmsYZ9YdQS9J1U5JRvMcJHdb9XBNvvEe9IpIg31sKNjIvX4 azlaBpgGl5wGvlFo3p+ayBIG+11kjgzo0KXTxnc++gpaHMXQ7mVhRPejMA0aD7GPQVGk ZNhWgY7dGGE43kLOhUvHZR6aw47Hg5w2oO1rmT2/l8keK7Un04dTHbDee76Y7HAWjsSf WPsg== X-Gm-Message-State: AOJu0YxQPex3Bxzo+zYT7gSlrc7v7aS6knwWJxBEezsk8Gc2cmB79+q6 Xh2rhzB5CcbgkiZGVCUNCqHUIeEX7PnrEbwtbTk30cQp8acO2FQWoG8UwheO9Ls= X-Google-Smtp-Source: AGHT+IFzIFNZgWHpVs+H3ueuARwxHmOyyGRoMAw1xYRuWFrmdam6GIhPfesjDLGWIlcw/53FBbbHnw== X-Received: by 2002:a17:907:3e9e:b0:a86:8f8f:4761 with SMTP id a640c23a62f3a-a8a885ff1cbmr879737866b.25.1725882251336; Mon, 09 Sep 2024 04:44:11 -0700 (PDT) Received: from [192.168.200.22] ([84.245.121.62]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a8d2583177dsm328763566b.30.2024.09.09.04.44.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 09 Sep 2024 04:44:10 -0700 (PDT) Message-ID: <3627c429-9a3e-48af-8c42-d85c83ee2998@pantheon.tech> Date: Mon, 9 Sep 2024 13:44:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/1] dts: add text parser for testpmd verbose output To: jspewock@iol.unh.edu, thomas@monjalon.net, yoan.picchi@foss.arm.com, paul.szczepanek@arm.com, Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, wathsala.vithanage@arm.com, Luca.Vizzarro@arm.com, npratte@iol.unh.edu, alex.chapman@arm.com Cc: dev@dpdk.org References: <20240729203955.267942-1-jspewock@iol.unh.edu> <20240808203612.329540-1-jspewock@iol.unh.edu> <20240808203612.329540-2-jspewock@iol.unh.edu> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: <20240808203612.329540-2-jspewock@iol.unh.edu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > diff --git a/dts/framework/parser.py b/dts/framework/parser.py > index 741dfff821..0b39025a48 100644 > --- a/dts/framework/parser.py > +++ b/dts/framework/parser.py > @@ -160,6 +160,36 @@ def _find(text: str) -> Any: > > return ParserFn(TextParser_fn=_find) > > + @staticmethod > + def find_all( > + pattern: str | re.Pattern[str], > + flags: re.RegexFlag = re.RegexFlag(0), > + ) -> ParserFn: I'd remove this if it's not used, the rule being let's not introduce unused code because it's not going to be maintained. We can always add it when needed. > + """Makes a parser function that finds all of the regular expression matches in the text. > + > + If there are no matches found in the text than None will be returned, otherwise a list then None, but maybe a comma would be better (found in the text, None will be returned) > + containing all matches will be returned. Patterns that contain multiple groups will pack > + the matches for each group into a tuple. > + > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py > index 43e9f56517..7d0b5a374c 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -31,7 +31,7 @@ > from framework.settings import SETTINGS > from framework.testbed_model.cpu import LogicalCoreCount, LogicalCoreList > from framework.testbed_model.sut_node import SutNode > -from framework.utils import StrEnum > +from framework.utils import REGEX_FOR_MAC_ADDRESS, StrEnum > > > class TestPmdDevice: > @@ -577,6 +577,377 @@ class TestPmdPortStats(TextParser): > tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)")) > > > +class OLFlag(Flag): We should come up with a consistent naming scheme for the various offloads. In the capabilities patch, I've introduced RxOffloadCapability. I think we can use the full word Offload and we should also capture in the name what sort of offload it is. In this case, would PacketOffloadFlag be a good name? > + """Flag representing the Packet Offload Features Flags in DPDK. > + > + Values in this class are taken from the definitions in the RTE MBUF core library in DPDK. I like the reference, let's also mention the name of the file rte_mbuf_core.h. Maybe we should add more references like these to other flags. > + """ > + > + # RX flags > + #: > + RTE_MBUF_F_RX_RSS_HASH = auto() > + > + #: I noticed the flags are not sorted the same way as in rte_mbuf_core.h. I think there's value in using the same flag values. We could also add descriptions to the flag if there are some to be found in rte_mbuf_core.h. > + > + # TX flags > + #: > + RTE_MBUF_F_TX_OUTER_UDP_CKSUM = auto() Since there is a gap between RX and TX flags, you can just assign the actual value here (1 << 41) and the continue using auto(). > + @classmethod > + def from_str_list(cls, arr: list[str]) -> Self: > + """Makes an instance from a list containing the flag members. > + > + Args: > + arr: A list of strings containing ol_flag values. > + > + Returns: > + A new instance of the flag. > + """ > + flag = cls(0) > + for name in arr: > + if name in cls.__members__: We could also do if cls[name] in cls. It's basically the same thing, but doesn't use the dunder method. > + flag |= cls[name] > + return flag > + > + @classmethod > + def make_parser(cls) -> ParserFn: > + """Makes a parser function. > + > + Returns: > + ParserFn: A dictionary for the `dataclasses.field` metadata argument containing a > + parser function that makes an instance of this flag from text. > + """ > + return TextParser.wrap( > + TextParser.wrap(TextParser.find(r"ol_flags: ([^\n]+)"), str.split), > + cls.from_str_list, > + ) > + > + > +class RtePTypes(Flag): > + """Flag representing possible packet types in DPDK verbose output.""" Now this docstring doesn't reference from where these come from. I found these in rte_mbuf_ptype.h, but from what I can tell, they're not actual flags, just regular numbers: #define RTE_PTYPE_L2_ETHER 0x00000001 #define RTE_PTYPE_L2_ETHER_TIMESYNC 0x00000002 etc., so we're basically converting that to flags. I think this is OK and we don't really need to concern ourselves with the actual values, just the order. > + @classmethod > + def from_str_list(cls, arr: list[str]) -> Self: > + """Makes an instance from a list containing the flag members. > + > + Args: > + arr: A list of strings containing ol_flag values. ol_flag looks like a copy-paste. > + > + Returns: > + A new instance of the flag. > + """ > + flag = cls(0) > + for name in arr: > + if name in cls.__members__: > + flag |= cls[name] > + return flag > + > + @classmethod > + def make_parser(cls, hw: bool) -> ParserFn: > + """Makes a parser function. > + > + Args: > + hw: Whether to make a parser for hardware ptypes or software ptypes. If :data:`True` I think there should be a comma before hardware (on the next line). > + hardware ptypes will be collected, otherwise software pytpes will. > + > + Returns: > + ParserFn: A dictionary for the `dataclasses.field` metadata argument containing a > + parser function that makes an instance of this flag from text. > + """ > + return TextParser.wrap( > + TextParser.wrap(TextParser.find(f"{'hw' if hw else 'sw'} ptype: ([^-]+)"), str.split), > + cls.from_str_list, > + ) > + > + > +@dataclass > +class TestPmdVerbosePacket(TextParser): > + ol_flags: OLFlag = field(metadata=OLFlag.make_parser()) > + #: RSS has of the packet in hex. typo: hash > class TestPmdShell(DPDKShell): > + @staticmethod > + def extract_verbose_output(output: str) -> list[TestPmdVerbosePacket]: > + """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`. > + """ > + out: list[TestPmdVerbosePacket] = [] > + prev_header: str = "" > + iter = re.finditer( > + r"(?P
(?:port \d+/queue \d+: received \d packets)?)\s*" Looks like sent packets won't be captured by this. > diff --git a/dts/framework/utils.py b/dts/framework/utils.py > index 6b5d5a805f..9c64cf497f 100644 > --- a/dts/framework/utils.py > +++ b/dts/framework/utils.py > @@ -27,6 +27,7 @@ > from .exception import ConfigurationError > > REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/" > +REGEX_FOR_MAC_ADDRESS: str = r"(?:[\da-fA-F]{2}:){5}[\da-fA-F]{2}" Is this the only format that testpmd returns?