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 F34A7456BF; Fri, 26 Jul 2024 22:18:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C49EB40ED6; Fri, 26 Jul 2024 22:18:27 +0200 (CEST) Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) by mails.dpdk.org (Postfix) with ESMTP id 4ACC340DD6 for ; Fri, 26 Jul 2024 22:18:27 +0200 (CEST) Received: by mail-pg1-f172.google.com with SMTP id 41be03b00d2f7-7a263f6439eso842439a12.3 for ; Fri, 26 Jul 2024 13:18:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1722025106; x=1722629906; 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=0O5FVw0Bv1AZat5APEM0faqSppw9rpdpyJdaVsUX+bI=; b=fcRAaxx6N9LqlvKEIl558sRxAEjoN/eDu8H2RbCQsGOVz6CDHlHr2tX8HKB4kIH2SB X9EZgDJzBmZAohWC0V4RgbUyxgs/1Z93qcMpHs3C1L7WuFJqcS341Yefcje5mIb0yHTn oc+3kC8coKfmNoYKKXkLUkeJrIBHB1FgytrR0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722025106; x=1722629906; 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=0O5FVw0Bv1AZat5APEM0faqSppw9rpdpyJdaVsUX+bI=; b=ZLvxTE5dRPeKfyAeFLnEVpjjpnJxBYq7P53iMn0oQc/kwuK3QhDDRr4ZS2fO117DtD FOK78lD2FxCF28PuHiKCe3cRUc6P9mXgMudAbKSE97EFMludHiGwQHD85m4KchO1Gpq+ v2gjvzADkZmUgFYhlgD0mdhymWwikDhKnsuA+CGu+wUaqUDyA8wA0AISlo7YXQEJKnyy XwjCb83VVSfqOJjiPRkzwdo6XUDzkT6DiIwM2MV1AzTtyW5gvwP9uT99/dW8Tm2Q1yjF wTDY/iFZPWvw33vYT1KoGv0a2/RQ4n98P2g7+7mrBkERLgUva4+Qt+PPxz3uY709oOHP yiIw== X-Forwarded-Encrypted: i=1; AJvYcCWSoPNMEFuKzZX8xlIhJFywWFnie9HykmIrlfQtQ5pCzoTJZ+kGHTeevyO7kBBzY/4fqlI2hfouAb9QPOo= X-Gm-Message-State: AOJu0Yz969OXe6pt8a2iu5UiPQUw+M8c+Z16jpi3ZjsF2nbmyA5Kc7D6 2mSUouIlv3/jAPGOAE6mEiY9I/Tk1m7K+bKRMDjxTJ/NIVngYl0k4huVCcjDbRWm7vjUlLHeoIU eDqB62IjjqiakzTrJZQIYKqdjlXeNDdrDId/mgA== X-Google-Smtp-Source: AGHT+IHU+ITiA0Zp7hHuDynsdPUGbHhIxKni29tAvJNSxDtOLmMyBnGmKR7Ekw13mfloBq+hX2ppbVRrrXon6NVd57M= X-Received: by 2002:a17:90b:1a92:b0:2c9:7e9e:70b3 with SMTP id 98e67ed59e1d1-2cf7e82c32amr678961a91.33.1722025106283; Fri, 26 Jul 2024 13:18:26 -0700 (PDT) MIME-Version: 1.0 References: <20240725162325.20933-1-dmarx@iol.unh.edu> <20240725162325.20933-2-dmarx@iol.unh.edu> In-Reply-To: <20240725162325.20933-2-dmarx@iol.unh.edu> From: Jeremy Spewock Date: Fri, 26 Jul 2024 16:18:15 -0400 Message-ID: Subject: Re: [RFC v1 1/3] dts: add UDP tunnel command to testpmd shell To: Dean Marx Cc: probb@iol.unh.edu, npratte@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 Hey Dean, these changes look good to me, I just had a few minor comments/suggestions. One thing I did notice was that the methods added here don't have type-hints for their return-types, obviously functionally it makes no difference since they don't return anything, but just adding the note that says the return None is helpful for type checkers and understanding the method at a glance. On Thu, Jul 25, 2024 at 12:23=E2=80=AFPM Dean Marx wrot= e: > > add udp_tunnel_port command to testpmd shell class, > also ports over set verbose method from vlan suite > > Signed-off-by: Dean Marx > --- > dts/framework/remote_session/testpmd_shell.py | 51 ++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor= k/remote_session/testpmd_shell.py > index eda6eb320f..26114091d6 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -804,7 +804,56 @@ def show_port_stats(self, port_id: int) -> TestPmdPo= rtStats: > > return TestPmdPortStats.parse(output) > > - def _close(self) -> None: It looks like this method might have been renamed by mistake in a rebase, the name on main right now is _close. This could cause some weird behavior in your testing since this is what the context manager uses to close the session, but I don't think it would have any drastic effect since the channel is still closed. > + def set_verbose(self, level: int, verify: bool =3D True): > + """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` the command output will be scanned 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 udp_tunnel_port( > + self, port_id: int, add: bool, udp_port: int, protocol: str, ver= ify: bool =3D True > + ): > + """Configures a UDP tunnel on the specified port, for the specif= ied protocol. > + > + Args: > + port_id: ID of the port to configure tunnel on. > + add: If :data:`True`, adds tunnel, otherwise removes tunnel. > + udp_port: ID of the UDP port to configure tunnel on. > + protocol: Name of tunnelling protocol to use; options are vx= lan, geneve, ecpri If there are explicit choices that this has to be like this it might be better to put these options into an enum and then pass that in as the parameter here. That way it is very clear from just calling the methods what your options are. > + verify: If :data:`True`, checks the output of the command to= verify that > + no errors were thrown. > + > + Raises: > + InteractiveCommandExecutionError: If verify is :data:`True` = and command > + output shows an error. > + """ > + action =3D "add" if add else "rm" > + cmd_output =3D self.send_command( > + f"port config {port_id} udp_tunnel_port {action} {protocol} = {udp_port}" > + ) > + if verify: > + if "Operation not supported" in cmd_output or "Bad arguments= " in cmd_output: > + self._logger.debug(f"Failed to set UDP tunnel: \n{cmd_ou= tput}") > + raise InteractiveCommandExecutionError(f"Failed to set U= DP tunnel: \n{cmd_output}") > + > + def close(self) -> None: > """Overrides :meth:`~.interactive_shell.close`.""" > self.stop() > self.send_command("quit", "Bye...") > -- > 2.44.0 >