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 54091457A5; Mon, 12 Aug 2024 22:32:24 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DB212402D3; Mon, 12 Aug 2024 22:32:23 +0200 (CEST) Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by mails.dpdk.org (Postfix) with ESMTP id 714F6402BB for ; Mon, 12 Aug 2024 22:32:22 +0200 (CEST) Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2ef260c4467so9733671fa.0 for ; Mon, 12 Aug 2024 13:32:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1723494742; x=1724099542; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ETmGum255rvCgFKz94STDzKBh3hAqA3uClkyEN1NDAI=; b=BRQvDJa4o5n47IGJzMHF/8H7eIPXI9lJrWH9ZmkmGyReFZrh8+SjHAdQ9zAolWL0af 3yY5Amm8zJXSedkAwWH7utMCF7yCyP5BJnkutVgWyiv+FY5OKgcRJySW1qmdrGUyUCU7 gIDplVBgylQ9BsBdXPL53fU6PIK+IcqPXb2pM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723494742; x=1724099542; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ETmGum255rvCgFKz94STDzKBh3hAqA3uClkyEN1NDAI=; b=w/nTdrax5mboOSX865QD7DxU5ZgEDCwYX6m8XmHpH6NszJrtMax6r3BBbygqky6v6p CpBMix4USblAvEr/nr/BcPpF+Yf7qIwN0cuBUI+ABx5y52uAhzLgHMkJOhEQmT/jR8XM aZtBKTNaAsLQEUAuL50AFSNP4qrQRz2a2sv3+EqazIyaDvTzCfMg5XH+5dwFUTg5wy3c F3q3HVd9h9CmbYKO7tWoNpuUcgbX08TcrvFfAYAUpq1BZQJ5Zy/NE1p72Z4XucB1Ucoq kVphchH0FmJuQuRhYeSoNXweAcdC75sR0A5a2W7GTWCx8UZaN35CKK983rKzBkshr96N dGsw== X-Forwarded-Encrypted: i=1; AJvYcCWtlb9vpQYiZKSuRatEq6PYXRFy+CIMS4cPmZirWlakXB3zvmlI3umZe3PYmIhhA6ACexw=@dpdk.org X-Gm-Message-State: AOJu0Yxndupq4Fzc31WTvVy/42eU++ycrBV4uSmyVcQnOeUIcMgIhlEU S5Mkn2E8aE0hBJsQgPXfsW8ovtKII+WuV33zcNrdDEs1BaeoWe05nbejj81MYwoeQKypEvBgxMi PZjP1FoWtWZsQ63OxsRn8CNJqB/AEMUy692XiyA== X-Google-Smtp-Source: AGHT+IFmRFnWxdmONG0WMtuTQ7pOXVGNKCDUrOFHUlqwJrbQWQdSNJW79Td7LBvCFtB38rv/nzNAFSKa3RKU3dj/Ktw= X-Received: by 2002:a05:651c:b06:b0:2ef:2373:5f90 with SMTP id 38308e7fff4ca-2f2ba92c1d3mr835081fa.0.1723494741468; Mon, 12 Aug 2024 13:32:21 -0700 (PDT) MIME-Version: 1.0 References: <20240812133936.26344-1-dmarx@iol.unh.edu> <20240812133936.26344-2-dmarx@iol.unh.edu> In-Reply-To: <20240812133936.26344-2-dmarx@iol.unh.edu> From: Nicholas Pratte Date: Mon, 12 Aug 2024 16:32:10 -0400 Message-ID: Subject: Re: [PATCH v1 1/2] dts: add csum HW offload to testpmd shell To: Dean Marx Cc: probb@iol.unh.edu, jspewock@iol.unh.edu, luca.vizzarro@arm.com, yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com, juraj.linkes@pantheon.tech, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Mon, Aug 12, 2024 at 9:39=E2=80=AFAM Dean Marx wrote= : > > add csum_set_hw method to testpmd shell class. Port over > set_verbose and port start/stop from queue start/stop suite. > > Signed-off-by: Dean Marx > --- > dts/framework/remote_session/testpmd_shell.py | 94 +++++++++++++++++++ > 1 file changed, 94 insertions(+) > > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor= k/remote_session/testpmd_shell.py > index 43e9f56517..be8fbdc295 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -806,6 +806,100 @@ def show_port_stats(self, port_id: int) -> TestPmdP= ortStats: > > return TestPmdPortStats.parse(output) > > + def port_stop(self, port: int, verify: bool =3D True): > + """Stop specified port. > + > + Args: > + port: Specifies the port number to use, must be between 0-32= . > + verify: If :data:`True`, the output of the command is scanne= d > + to ensure specified port is stopped. If not, it is consi= dered > + an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and the port > + is not stopped.""" > + port_output =3D self.send_command(f"port stop {port}") > + if verify: > + if "Done" not in port_output: > + self._logger.debug(f"Failed to stop port {port}: \n{port= _output}") > + raise InteractiveCommandExecutionError(f"Testpmd failed = to stop port {port}.") > + > + def port_start(self, port: int, verify: bool =3D True): > + """Start specified port. > + > + Args: > + port: Specifies the port number to use, must be between 0-32= . > + verify: If :data:`True`, the output of the command is scanne= d > + to ensure specified port is started. If not, it is consi= dered > + an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and the port > + is not started.""" > + port_output =3D self.send_command(f"port start {port}") > + if verify: > + if "Done" not in port_output: > + self._logger.debug(f"Failed to start port {port}: \n{por= t_output}") > + raise InteractiveCommandExecutionError(f"Testpmd failed = to start port {port}.") > + > + def csum_set_hw(self, layer: str, port_id: int, verify: bool =3D Tru= e) -> None: > + """Enables hardware checksum offloading on the specified layer. > + > + Args: > + layer: The layer that checksum offloading should be enabled = on. > + options: tcp, ip, udp, sctp, outer-ip, outer-udp. > + port_id: The port number to enable checksum offloading on, s= hould be within 0-32. > + verify: If :data:`True` the output of the command will be sc= anned in an attempt to > + verify that checksum offloading was enabled on the port. I ran into a similar situation with the VLAN offload set command which requires a string input much like that of the 'layer' parameter here. While this would technically work, it might be best to create some kind of type class (or even Flags if at all possible) so that users can select from a more rigorous list of options when writing test suites. For example, instead of passing a string into the method, you have the user pass in something like csum_set_hw(LayerTypes.ip, port_id =3D 0) where 'LayerTypes' is some kind of Flag or user-defined type class. It's really just a preference, but I do think it makes for more concise code. > + > + Raises: > + InteractiveCommandExecutionError: If checksum offload is not= enabled successfully. > + """ > + csum_output =3D self.send_command(f"csum set {layer} hw {port_id= }") > + if (verify and ("Bad arguments" in csum_output or f"Please stop = port {port_id} first")): > + self._logger.debug(f"Failed to set csum hw mode on port {por= t_id}:\n{csum_output}") > + raise InteractiveCommandExecutionError( > + f"Failed to set csum hw mode on port {port_id}" > + ) > + if verify and f"checksum offload is not supported by port {port_= id}" in csum_output: > + self._logger.debug(f"Checksum {layer} offload is not support= ed by port {port_id}:\n{csum_output}") > + > + success =3D False > + if "outer-ip" in layer: > + if "Outer-Ip checksum offload is hw" in csum_output: > + success =3D True > + if "outer-udp" in layer: > + if "Outer-Udp checksum offload is hw" in csum_output: > + success =3D True > + else: > + if f"{layer.upper} checksum offload is hw" in csum_output: > + success =3D True > + if not success and verify: > + self._logger.debug(f"Failed to set csum hw mode on port {por= t_id}:\n{csum_output}") > + > + def set_verbose(self, level: int, verify: bool =3D True) -> None: > + """Set debug verbosity level. > + > + Args: > + level: 0 - silent except for error > + 1 - fully verbose except for Tx packets > + 2 - fully verbose except for Rx packets > + >2 - fully verbose > + verify: if :data:`True` an additional command will be sent t= o verify that verbose level > + is properly set. Defaults to :data:`True`. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and verbose level > + is not correctly set. > + """ > + verbose_output =3D self.send_command(f"set verbose {level}") > + if verify: > + if "Change verbose level" not in verbose_output: > + self._logger.debug(f"Failed to set verbose level to {lev= el}: \n{verbose_output}") > + raise InteractiveCommandExecutionError( > + f"Testpmd failed to set verbose level to {level}." > + ) > + > def _close(self) -> None: > """Overrides :meth:`~.interactive_shell.close`.""" > self.stop() > -- > 2.44.0 >